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

[SNOW-81] Add comments to certifiedquiz_latest table and columns #48

Merged
merged 1 commit into from
Jun 10, 2024

Conversation

philerooski
Copy link
Collaborator

No description provided.

@philerooski philerooski requested a review from a team as a code owner June 5, 2024 19:53
@@ -0,0 +1,17 @@
USE SCHEMA {{database_name}}.synapse; --noqa: JJ01,PRS,TMP

COMMENT ON TABLE CERTIFIEDQUIZ_LATEST IS 'This table contain snapshots of submissions of user verification data by ACT. Snapshots are taken when a submission is created or updated. Note: Snapshots are also taken periodically and independently of the changes. The snapshot_timestamp records when the snapshot was taken.';
Copy link
Contributor

@BryanFauble BryanFauble Jun 5, 2024

Choose a reason for hiding this comment

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

The format for this is slightly different, example:
ALTER TABLE CERTIFIEDQUIZQUESTION_LATEST SET COMMENT = 'The latest snapshot of the questions of the certification quiz. With each entry representing a question answered by the user during the quiz.';

Suggested change
COMMENT ON TABLE CERTIFIEDQUIZ_LATEST IS 'This table contain snapshots of submissions of user verification data by ACT. Snapshots are taken when a submission is created or updated. Note: Snapshots are also taken periodically and independently of the changes. The snapshot_timestamp records when the snapshot was taken.';
ALTER TABLE CERTIFIEDQUIZ_LATEST SET COMMENT = 'This table contain snapshots of submissions of user verification data by ACT. Snapshots are taken when a submission is created or updated. Note: Snapshots are also taken periodically and independently of the changes. The snapshot_timestamp records when the snapshot was taken.';

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Am I misunderstanding the docs?

Comments can be added to all objects (users, roles, warehouses, databases, tables, etc.).
COMMENT [IF EXISTS] ON <object_type> <object_name> IS '<string_literal>';

The docs mention your method as an alternative:

In addition to this command, a comment can be added when creating or altering an object:

Copy link
Contributor

Choose a reason for hiding this comment

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

You're right @philerooski - Your way of doing it looks like one of the possible ways of doing it.

Copy link
Member

Choose a reason for hiding this comment

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

@philerooski can you update the table description here? This is the "latest" table, the description you have is for the snapshot table

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@thomasyu888 The schema file you linked to in the ticket doesn't have a certifiedquizlatest schema. Could you link me to the correct schema or describe how this table is different from the snapshot table?

Copy link
Member

@thomasyu888 thomasyu888 Jun 10, 2024

Choose a reason for hiding this comment

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

@philerooski Sorry for the confusion. This latest table is a de-duplicated version of the snapshot table (latest snapshot). The description that you pulled from the schema is the snapshot tables, which contains snapshots per week and per event.

We write something like this to define the latest tables: https://github.com/Sage-Bionetworks/snowflake/blob/dev/synapse_data_warehouse/synapse/dynamic_tables/V2.17.0__fileassociation_team_latest.sql.

So what @BryanFauble suggested:
"The latest snapshot of the questions of the certification quiz. With each entry representing a question answered by the user during the quiz.'"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@thomasyu888 Thanks. I've updated the description. I also modified the comment on column snapshot_date because it mentioned about being a partition in the snapshot table, which isn't the case in the latest table.

Copy link
Contributor

@BryanFauble BryanFauble left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

Copy link
Member

@thomasyu888 thomasyu888 left a comment

Choose a reason for hiding this comment

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

🔥 LGTM!

@thomasyu888 thomasyu888 merged commit f55b103 into dev Jun 10, 2024
3 checks passed
@thomasyu888 thomasyu888 deleted the snow-81 branch June 10, 2024 19:59
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.

3 participants