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

feat: support annotations in SQL file #171

Merged
merged 7 commits into from
Sep 27, 2024

Conversation

gurminder71
Copy link
Contributor

Spanner DDL SQL is often checked into source control. During the CI build there is often need to:

  • Generate delta DDL and upgrade the Spanner schema. This tool already supports it
  • Use the Spanner schema information in other build steps

Since Spanner DDL is an important source of truth, it useful to annotate the schema with meta information. One such use case to to support annotations on table columns like:

CREATE TABLE test1 (
  -- @ANNOTATION DEPRECATED,
  -- @ANNOTATION PII,
  -- @ANNOTATION PII.LEVEL_1(key1=val1,key2=value),
  id STRING(36),
) PRIMARY KEY (id)

This concept is similar to Java annotations where tools like javadoc can generated documentation based on the annotations. Some of the use cases of annotations:

  • Mark a column deprecated
  • Mark the column with PII tags

Since the @ANNOTATION is not a valid SQL statement, it is added as comment in the file. The annotations are parsed into AST and will become part of DatabaseDefinition. There is no delta generation as Spanner does not understand annotations or does not even support comments.

The CI build steps can use table column annotation.

@gurminder71 gurminder71 changed the title (feat): support annotations in SQL file feat: support annotations in SQL file Sep 24, 2024
@nielm
Copy link
Collaborator

nielm commented Sep 24, 2024

Can you add some examples in the 3 files in the test resources -

  • originalDdl.txt
  • newDdl.txt
  • expedtedDdlDiff.txt

To verify that these annotations do not break parsing/diff generation - eg changing annotations without changing table definition is a no-op

https://github.com/cloudspannerecosystem/spanner-schema-diff-tool/tree/main/src/test/resources

@gurminder71
Copy link
Contributor Author

Added test case to ensure that annotations do not result in diff generation

@nielm
Copy link
Collaborator

nielm commented Sep 26, 2024

I have updated the annotations in the test files to have the comment prefix...

@gurminder71
Copy link
Contributor Author

Initially I though of adding --enableAnnotations as command line parameter to DdlDiff. But there is no use case of annotations in diff generation. So always skipping parsing the annotations in diff generation.

The export of the parsed DDL is an entirely new feature. The README.md may not be right file to advertise this capability. An additional markdown file may be added to advertise this new capability to have the parse statements available to other Java programs. I am leaving the advertisement to you.

@nielm
Copy link
Collaborator

nielm commented Sep 27, 2024

Initially I though of adding --enableAnnotations as command line parameter to DdlDiff. But there is no use case of annotations in diff generation. So always skipping parsing the annotations in diff generation.

Yep thats fine.

The export of the parsed DDL is an entirely new feature. The README.md may not be right file to advertise this capability. An additional markdown file may be added to advertise this new capability to have the parse statements available to other Java programs. I am leaving the advertisement to you.

I have added a comment in the parser jjt code and annotations AST class that this Annotations are not a Spanner feature, but can be used by customers who wish to add metadata to column definitions and use this parser...

@nielm nielm merged commit 63efbd6 into cloudspannerecosystem:main Sep 27, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants