-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
API: Reject unknown type for required fields and validate defaults #12302
API: Reject unknown type for required fields and validate defaults #12302
Conversation
2f28c11
to
cfe64b8
Compare
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.
LGTM
This one was also on my list as well after reviewing #12211
@@ -694,7 +698,10 @@ private static Literal<?> castDefault(Literal<?> defaultValue, Type type) { | |||
throw new IllegalArgumentException( | |||
String.format("Invalid default value for %s: %s (must be null)", type, defaultValue)); | |||
} else if (defaultValue != null) { | |||
return defaultValue.to(type); | |||
Literal<?> typedDefault = defaultValue.to(type); |
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.
Thinking of it, would a NoneLiteral
make sense 🤔
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.
Right now, we make a lot of assumptions that Literal does not contain null or NaN. That way, we don't need to implement 3-value boolean logic or worry about how to handle NaN in comparisons.
If you want to have a null default, then you don't set a default value for an optional column. And this also passes through null without a problem if the Literal was null.
Co-authored-by: Eduard Tudenhoefner <etudenhoefner@gmail.com>
* Site: Learn More to point to Spark QuickStart Doc (apache#12272) * Build: Bump datamodel-code-generator from 0.27.2 to 0.28.1 (apache#12290) * Spark 3.5: Fix job description of RewriteTablePathSparkAction (apache#12282) * Build: Bump io.netty:netty-buffer from 4.1.117.Final to 4.1.118.Final (apache#12287) Bumps [io.netty:netty-buffer](https://github.com/netty/netty) from 4.1.117.Final to 4.1.118.Final. - [Commits](netty/netty@netty-4.1.117.Final...netty-4.1.118.Final) --- updated-dependencies: - dependency-name: io.netty:netty-buffer dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Build: Bump software.amazon.awssdk:bom from 2.30.16 to 2.30.21 (apache#12286) Bumps software.amazon.awssdk:bom from 2.30.16 to 2.30.21. --- updated-dependencies: - dependency-name: software.amazon.awssdk:bom dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * OpenAPI: Add overwrite option when registering a table (apache#12239) * OpenAPI: Add optional overwrite when registering table * simplify to overwrite * Add the article to the description Co-authored-by: Eduard Tudenhoefner <etudenhoefner@gmail.com> * Update generated python as well Signed-off-by: Hongyue Zhang <steveiszhy@gmail.com> * Fix import order --------- Signed-off-by: Hongyue Zhang <steveiszhy@gmail.com> Co-authored-by: Eduard Tudenhoefner <etudenhoefner@gmail.com> * Build: Bump mkdocs-material from 9.6.3 to 9.6.4 (apache#12284) Bumps [mkdocs-material](https://github.com/squidfunk/mkdocs-material) from 9.6.3 to 9.6.4. - [Release notes](https://github.com/squidfunk/mkdocs-material/releases) - [Changelog](https://github.com/squidfunk/mkdocs-material/blob/master/CHANGELOG) - [Commits](squidfunk/mkdocs-material@9.6.3...9.6.4) --- updated-dependencies: - dependency-name: mkdocs-material dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Core: Fix Enabling row-lineage during Create Table (apache#12307) * API: Reject unknown type for required fields and validate defaults (apache#12302) * API: Fix TestInclusiveMetricsEvaluator notStartsWith tests. (apache#12303) * Core: Add variant type support to utils and visitors (apache#11831) * Core: Fix CI: Update tests with UnknownType from required to optional (apache#12316) * Docs: Refactor site navigation bar (apache#12289) * Parquet: Implement Variant readers (apache#12139) * Docs: Add rewrite_table_path Spark Procedure (apache#12115) * Parquet: Fix errorprone warning (apache#12324) * Docs: Add Apache Amoro docs (apache#11966) * Parquet: Fix performance regression in reader init (apache#12305) * Core: Fallback to GET requests for namespace/table/view exists checks (apache#12314) Co-authored-by: Daniel Weeks <dweeks@apache.org> * Docs: Fix refs in Apache Amoro docs (apache#12332) * Revert "Core: Serialize `null` when there is no current snapshot (apache#11560)" (apache#12312) This reverts commit bf8d25f. * Parquet: Fix performance regression in reader init (apache#12305) (apache#12329) Co-authored-by: Bryan Keller <bryanck@gmail.com> * Checkstyle: Apply the same generic type naming rules to interfaces and classes (apache#12333) --------- Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: Hongyue Zhang <steveiszhy@gmail.com> Co-authored-by: Danica Fine <danica.fine@gmail.com> Co-authored-by: Manu Zhang <OwenZhang1990@gmail.com> Co-authored-by: Yuya Ebihara <ebyhry@gmail.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Hongyue/Steve Zhang <steveiszhy@gmail.com> Co-authored-by: Eduard Tudenhoefner <etudenhoefner@gmail.com> Co-authored-by: Tom Tanaka <43331405+tomtongue@users.noreply.github.com> Co-authored-by: Ryan Blue <blue@apache.org> Co-authored-by: Aihua Xu <aihuaxu@gmail.com> Co-authored-by: Fokko Driesprong <fokko@apache.org> Co-authored-by: ConradJam <jam.gzczy@gmail.com> Co-authored-by: Bryan Keller <bryanck@gmail.com> Co-authored-by: Daniel Weeks <dweeks@apache.org> Co-authored-by: pvary <peter.vary.apache@gmail.com>
This updates
NestedField
to check thatUnknownType
is not used in a required field, which is not permitted by the spec. It also checks that default values are cast to the correct type or are rejected.