-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Add test to ensure consistency of generated column and CDF in Delta #14629
Conversation
398c6f1
to
2882103
Compare
1943ead
to
7587828
Compare
String tableDirectory = "databricks-compatibility-test-" + tableName; | ||
|
||
onDelta().executeQuery(format("" + | ||
"CREATE TABLE default.%s (col int) " + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: you can use multiline string literal
onTrino().executeQuery("COMMENT ON TABLE delta.default." + tableName + " IS 'test table comment'"); | ||
onTrino().executeQuery("ALTER TABLE delta.default." + tableName + " ADD COLUMN new_column INT"); | ||
|
||
List<?> enableChangeDataFeed = getOnlyElement(onDelta().executeQuery("SHOW TBLPROPERTIES " + tableName + "(delta.enableChangeDataFeed)").rows()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getOnlyElement -> getOnlyValue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can't use getOnlyValue
because SHOW TBLPROPERTIES
returns two columns (key
& value
).
onTrino().executeQuery("ALTER TABLE delta.default." + tableName + " ADD COLUMN new_column INT"); | ||
|
||
List<?> enableChangeDataFeed = getOnlyElement(onDelta().executeQuery("SHOW TBLPROPERTIES " + tableName + "(delta.enableChangeDataFeed)").rows()); | ||
assertTrue(Boolean.parseBoolean((String) enableChangeDataFeed.get(1))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
try to use assertj so that enableChangeDataFeed value is provided (original, before parsing) when the assertion fails
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to getOnlyElement
+ QueryResult#column
to improve the readability.
String tableDirectory = "databricks-compatibility-test-" + tableName; | ||
|
||
onDelta().executeQuery(format("" + | ||
"CREATE TABLE default.%s (a INT, b BOOLEAN GENERATED ALWAYS AS (CAST(true AS BOOLEAN))) " + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is the CAST required?
Would it make sense to have a slightly more complicated case. where you generate b
from a
value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to b INT GENERATED ALWAYS AS (a * 2)
instead.
f5b4be1
to
6c92d3b
Compare
6c92d3b
to
667d842
Compare
Description
Add test to ensure consistency of generated column in Delta
Relates to #12637
Release notes
(x) This is not user-visible or docs only and no release notes are required.