Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Avro Doc Comments materialize_sink_kafka #373

Merged
merged 7 commits into from
Dec 12, 2023
Merged

Conversation

dehume
Copy link
Contributor

@dehume dehume commented Nov 20, 2023

Fixes #359

Add two new attributes to the materialize_sink_kafka, avro_doc_type and avro_doc_column to support Avro comments. Adds the following syntax to the SQL

CREATE SINK avro_sink
  ...
  (
    DOC ON TYPE t = 'top-level comment for avro record in both key and value schemas',
    KEY DOC ON COLUMN t.c1 = 'comment on column only in key schema',
    VALUE DOC ON COLUMN t.c1 = 'comment on column only in value schema'
  )

@dehume dehume added the enhancement New feature or request label Nov 20, 2023
@benesch
Copy link
Contributor

benesch commented Nov 21, 2023

This is still WIP, so let me know if this is too deep too soon, but I think we should track the SQL syntax more closely here. I think you want something more like:

format {
    avro {
        doc_on_type {
            database_name = "..."
            schema_name = "..."
            type_name = "..."
            doc = "..."
            # At least one of key or value must be true.
            key = {true | false}
            value = {true | false}
        }

        doc_on_column {
            database_name = "..."
            schema_name = "..."
            item_name = "..."
            column_name = "..."
            doc = "..."
            # At least one of key or value must be true.
            key = {true | false}
            value = {true | false}
        }
    }
}

@dehume dehume changed the title [WIP] Avro Comments Avro Comments Nov 29, 2023
@dehume dehume changed the title Avro Comments Support Avro Doc Comments materialize_sink_kafka Nov 29, 2023
@dehume dehume marked this pull request as ready for review November 29, 2023 23:10
@dehume dehume requested a review from bobbyiliev as a code owner November 29, 2023 23:10
c.WriteString("VALUE ")
}
c.WriteString(fmt.Sprintf("DOC ON TYPE %[1]s = %[2]s",
b.format.Avro.DocType.Object.QualifiedName(),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on docs, unsure if we need to also support aliases for the object reference. Or if it should just always make the connection.

@@ -285,6 +285,66 @@ func SinkFormatSpecSchema(elem string, description string, required bool) *schem
Optional: true,
ForceNew: true,
},
"avro_doc_type": {
Description: "**Private Preview** Add top level documentation comment to the generated Avro schemas.",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Open to better descriptions for all these.

@dehume dehume marked this pull request as draft November 29, 2023 23:14
@dehume dehume marked this pull request as ready for review November 30, 2023 13:03
@dehume
Copy link
Contributor Author

dehume commented Nov 30, 2023

The acceptance test failing is within Kafka Sources, not sinks. Must be due to some of the shared structs within format_specs

@dehume dehume merged commit 5d2c864 into main Dec 12, 2023
5 checks passed
@dehume dehume deleted the Avro-Comments branch December 12, 2023 11:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Kafka sink: support DOC ON options
3 participants