You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "singhpk234 (via GitHub)" <gi...@apache.org> on 2026/07/08 18:50:07 UTC
[PR] OpenAPI: Update REST spec expressions to match new expressions spec [iceberg]
singhpk234 opened a new pull request, #17138:
URL: https://github.com/apache/iceberg/pull/17138
(no comment)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: Update REST spec expressions to match new expressions spec [iceberg]
Posted by "nevzheng (via GitHub)" <gi...@apache.org>.
nevzheng commented on code in PR #17138:
URL: https://github.com/apache/iceberg/pull/17138#discussion_r3562687343
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2707,9 +2703,8 @@ components:
Literals:
description: >
- A set of literal values per Iceberg Expressions spec, Appendix B.
- Two JSON forms: a bare array of literals, or a typed object with an
- explicit data-type applied to all values.
+ A list of literal values. Two JSON forms: a bare array of literals,
Review Comment:
how about: `A Literal is either a bare value, a value object, or a typed value object`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: Update REST spec expressions to match new expressions spec [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #17138:
URL: https://github.com/apache/iceberg/pull/17138#discussion_r3591167518
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2646,20 +2878,37 @@ components:
term:
$ref: '#/components/schemas/Term'
values:
- type: array
- items:
- $ref: '#/components/schemas/PrimitiveTypeValue'
+ $ref: '#/components/schemas/Literals'
Term:
oneOf:
- $ref: '#/components/schemas/Reference'
- $ref: '#/components/schemas/TransformTerm'
+ - $ref: '#/components/schemas/DeprecatedReference'
Reference:
type: string
example:
- "column-name"
+ DeprecatedReference:
Review Comment:
Also, we may want to use `Reference` as the name instead of `FieldReference`, so maybe we do want a `DeprecatedReference`. But I think we only want 2 references (deprecated, field reference) rather than 3 (deprecated, reference, field reference).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: Update REST spec expressions to match new expressions spec [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #17138:
URL: https://github.com/apache/iceberg/pull/17138#discussion_r3618225907
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2528,15 +2528,16 @@ components:
items:
type: integer
- Expression:
+ Predicate:
oneOf:
+ - type: boolean
- $ref: '#/components/schemas/TrueExpression'
- $ref: '#/components/schemas/FalseExpression'
- - $ref: '#/components/schemas/AndOrExpression'
- - $ref: '#/components/schemas/NotExpression'
- - $ref: '#/components/schemas/SetExpression'
- - $ref: '#/components/schemas/LiteralExpression'
- - $ref: '#/components/schemas/UnaryExpression'
+ - $ref: '#/components/schemas/AndOrPredicate'
+ - $ref: '#/components/schemas/NotPredicate'
+ - $ref: '#/components/schemas/UnaryPredicate'
Review Comment:
Minor: Not is unary, so it may be helpful to have a different name for this. In the spec I used "Test"
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2528,15 +2528,16 @@ components:
items:
type: integer
- Expression:
+ Predicate:
oneOf:
+ - type: boolean
- $ref: '#/components/schemas/TrueExpression'
- $ref: '#/components/schemas/FalseExpression'
- - $ref: '#/components/schemas/AndOrExpression'
- - $ref: '#/components/schemas/NotExpression'
- - $ref: '#/components/schemas/SetExpression'
- - $ref: '#/components/schemas/LiteralExpression'
- - $ref: '#/components/schemas/UnaryExpression'
+ - $ref: '#/components/schemas/AndOrPredicate'
+ - $ref: '#/components/schemas/NotPredicate'
+ - $ref: '#/components/schemas/UnaryPredicate'
Review Comment:
Minor: Not is unary, so it may be helpful to have a different name for this. In the spec I used "Test"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: Update REST spec expressions to match new expressions spec [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #17138:
URL: https://github.com/apache/iceberg/pull/17138#discussion_r3591183648
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2606,7 +2630,194 @@ components:
child:
$ref: '#/components/schemas/Expression'
+ UnaryPredicate:
+ description: >
+ A predicate that tests a single value expression. Replaces the deprecated
+ UnaryExpression that used 'term'.
+ type: object
+ required:
+ - type
+ - child
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["is-null", "not-null", "is-nan", "not-nan"]
+ child:
+ $ref: '#/components/schemas/ValueExpression'
+
+ ComparisonPredicate:
+ description: >
+ A predicate that compares two value expressions. Replaces the deprecated
+ LiteralExpression that used 'term' and 'value'.
+ type: object
+ required:
+ - type
+ - left
+ - right
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["lt", "lt-eq", "gt", "gt-eq", "eq", "not-eq", "starts-with", "not-starts-with"]
+ left:
+ $ref: '#/components/schemas/ValueExpression'
+ right:
+ $ref: '#/components/schemas/ValueExpression'
+
+ SetPredicate:
+ description: >
+ A predicate that tests whether a value expression is in a set of literals.
+ Replaces the deprecated SetExpression that used 'term'.
+ type: object
+ required:
+ - type
+ - child
+ - values
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["in", "not-in"]
+ child:
+ $ref: '#/components/schemas/ValueExpression'
+ values:
+ $ref: '#/components/schemas/Literals'
+
+ Literal:
+ description: >
+ A literal value expression per Iceberg Expressions spec, Appendix B.
+ Three JSON forms are accepted: a bare scalar value, a typed literal
+ object without a data-type, or a typed literal object with an
+ explicit data-type.
+ oneOf:
+ - type: string
+ - type: number
+ - type: integer
+ - type: boolean
+ - type: 'null'
+ - type: object
+ required:
+ - type
+ - value
+ properties:
+ type:
+ type: string
+ const: "literal"
+ value:
+ $ref: '#/components/schemas/PrimitiveTypeValue'
+ data-type:
+ $ref: '#/components/schemas/PrimitiveType'
+
+ Literals:
+ description: >
+ A set of literal values per Iceberg Expressions spec, Appendix B.
+ Two JSON forms: a bare array of literals, or a typed object with an
+ explicit data-type applied to all values.
+ oneOf:
+ - type: array
+ items:
+ $ref: '#/components/schemas/Literal'
+ - type: object
+ required:
+ - type
+ - values
+ properties:
+ type:
+ type: string
+ const: "literals"
+ values:
+ type: array
+ items:
+ $ref: '#/components/schemas/PrimitiveTypeValue'
+ data-type:
+ $ref: '#/components/schemas/PrimitiveType'
+
+ FieldReference:
Review Comment:
> This allows expressions to specify which variant they accept.
I don't think this is correct. We don't have a `ValueReference` that is limited to `FieldReferenceById` or `FieldReferenceByName`. It would be hard to add one given that we would have to have similarly restricted predicate types. That would create 2 trees (predicates and value expressions with ID references, predicates and value expressions with named references) as well as types that allow both references.
I think it is better to approach this problem by specifying whether a given expression must use only ID or name references in its documentation rather than making this spec quite a bit larger to make it structural.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: Update REST spec expressions to match new expressions spec [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #17138:
URL: https://github.com/apache/iceberg/pull/17138#discussion_r3591057463
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2606,7 +2630,205 @@ components:
child:
$ref: '#/components/schemas/Expression'
+ UnaryPredicate:
+ description: >
+ A predicate that tests a single value expression.
+ type: object
+ additionalProperties: false
+ required:
+ - type
+ - child
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["is-null", "not-null", "is-nan", "not-nan"]
+ child:
+ $ref: '#/components/schemas/ValueExpression'
+
+ ComparisonPredicate:
+ description: >
+ A predicate that compares two value expressions.
+ type: object
+ additionalProperties: false
+ required:
+ - type
+ - left
+ - right
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["lt", "lt-eq", "gt", "gt-eq", "eq", "not-eq", "starts-with", "not-starts-with"]
+ left:
Review Comment:
Similar to the other cases, I think we should allow `term` and `value` here, since those are more strict than `ValueExpression`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: Update REST spec expressions to match new expressions spec [iceberg]
Posted by "nevzheng (via GitHub)" <gi...@apache.org>.
nevzheng commented on code in PR #17138:
URL: https://github.com/apache/iceberg/pull/17138#discussion_r3562674387
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2787,10 +2787,9 @@ components:
FunctionReference:
description: >
- A reference to a function per Iceberg Expressions spec, Appendix B.
- Four JSON forms: a bare name (single-part identifier), a list of
- names (multi-part identifier), an object with an identifier, or an
- object with both a catalog and an identifier.
+ A reference to a function. Four JSON forms: a bare name (single-part
Review Comment:
I notice "JSON forms" across the description. It feels a bit clunky to me.
I would try to establish a consistent convention across descriptions. `X is a/either/or x, y, z` (something like this maybe). and try to refer to the parts with meaning.
ex: `A function reference is either a name, a multi-part name, an identifier object, or a catalog-qualified identifier object. `
the goal is to concise.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: Update REST spec expressions to match new expressions spec [iceberg]
Posted by "nevzheng (via GitHub)" <gi...@apache.org>.
nevzheng commented on PR #17138:
URL: https://github.com/apache/iceberg/pull/17138#issuecomment-4930940655
Following up on an earlier thought. If possible, I'd recommend following the grammar as literally as OpenAPI allows. I hope this will be clearer, and save us from misinterpretations, and back-and-forth time.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: Update REST spec expressions to match new expressions spec [iceberg]
Posted by "huaxingao (via GitHub)" <gi...@apache.org>.
huaxingao commented on code in PR #17138:
URL: https://github.com/apache/iceberg/pull/17138#discussion_r3564849728
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2606,7 +2630,206 @@ components:
child:
$ref: '#/components/schemas/Expression'
+ UnaryPredicate:
+ description: >
+ A predicate that tests a single value expression.
+ type: object
Review Comment:
Should we add `additionalProperties: false` to `UnaryPredicate`, `ComparisonPredicate`, and `SetPredicate`? The new value schemas (`Literal`, `FieldReference*`, `Apply`) already have it.
Right now these predicates allow extra fields. So an object that has both a new field (child) and an old field (term) would match both the new predicate and the old deprecated one. Since `Expression` uses `oneOf`, matching two schemas at once is an error. Adding `additionalProperties: false` makes each schema reject fields it doesn't expect, so only one can match.
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2606,7 +2630,206 @@ components:
child:
$ref: '#/components/schemas/Expression'
+ UnaryPredicate:
+ description: >
+ A predicate that tests a single value expression.
+ type: object
+ required:
+ - type
+ - child
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["is-null", "not-null", "is-nan", "not-nan"]
+ child:
+ $ref: '#/components/schemas/ValueExpression'
+
+ ComparisonPredicate:
+ description: >
+ A predicate that compares two value expressions.
+ type: object
+ required:
+ - type
+ - left
+ - right
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["lt", "lt-eq", "gt", "gt-eq", "eq", "not-eq", "starts-with", "not-starts-with"]
+ left:
+ $ref: '#/components/schemas/ValueExpression'
+ right:
+ $ref: '#/components/schemas/ValueExpression'
+
+ SetPredicate:
+ description: >
+ A predicate that operates on a list of literals.
+ type: object
+ required:
+ - type
+ - child
+ - values
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["in", "not-in"]
+ child:
+ $ref: '#/components/schemas/ValueExpression'
+ values:
+ $ref: '#/components/schemas/Literals'
+
+ Literal:
+ description: >
+ A literal value expression. Three JSON forms are accepted: a bare
+ scalar value, a typed literal object without a data-type, or a
+ typed literal object with an explicit data-type.
+ oneOf:
+ - type: string
+ - type: number
+ - type: boolean
Review Comment:
The bare-value branch uses `string` | `number` | `boolean` while the typed-object branch reuses `PrimitiveTypeValue` for value , is the split intentional?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: Update REST spec expressions to match new expressions spec [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #17138:
URL: https://github.com/apache/iceberg/pull/17138#discussion_r3618273535
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2673,7 +2834,7 @@ components:
transform:
$ref: '#/components/schemas/Transform'
term:
- $ref: '#/components/schemas/Reference'
+ $ref: '#/components/schemas/Term'
Review Comment:
I don't think this is correct because it allows the inner term to be another transform. It should be `TermReference` instead to ensure that a transform always has a direct name reference for its `term`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: Update REST spec expressions to match new expressions spec [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #17138:
URL: https://github.com/apache/iceberg/pull/17138#discussion_r3641318511
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2528,15 +2528,16 @@ components:
items:
type: integer
- Expression:
+ Predicate:
Review Comment:
@danielcweeks, these names are internal and are basically reference labels. We don't break anything by changing them, and it means these definitions can that match the expressions spec more closely.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: Update REST spec expressions to match new expressions spec [iceberg]
Posted by "singhpk234 (via GitHub)" <gi...@apache.org>.
singhpk234 commented on PR #17138:
URL: https://github.com/apache/iceberg/pull/17138#issuecomment-5095468213
Thanks everyone for the review !
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: Update REST spec expressions to match new expressions spec [iceberg]
Posted by "singhpk234 (via GitHub)" <gi...@apache.org>.
singhpk234 merged PR #17138:
URL: https://github.com/apache/iceberg/pull/17138
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: Update REST spec expressions to match new expressions spec [iceberg]
Posted by "nevzheng (via GitHub)" <gi...@apache.org>.
nevzheng commented on code in PR #17138:
URL: https://github.com/apache/iceberg/pull/17138#discussion_r3562689707
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2707,9 +2703,8 @@ components:
Literals:
description: >
- A set of literal values per Iceberg Expressions spec, Appendix B.
- Two JSON forms: a bare array of literals, or a typed object with an
- explicit data-type applied to all values.
+ A list of literal values. Two JSON forms: a bare array of literals,
Review Comment:
LITERALS is either a list of literals or a typed list of values.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: Update REST spec expressions to match new expressions spec [iceberg]
Posted by "nevzheng (via GitHub)" <gi...@apache.org>.
nevzheng commented on code in PR #17138:
URL: https://github.com/apache/iceberg/pull/17138#discussion_r3555387422
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2606,7 +2630,194 @@ components:
child:
$ref: '#/components/schemas/Expression'
+ UnaryPredicate:
+ description: >
+ A predicate that tests a single value expression. Replaces the deprecated
+ UnaryExpression that used 'term'.
+ type: object
+ required:
+ - type
+ - child
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["is-null", "not-null", "is-nan", "not-nan"]
+ child:
+ $ref: '#/components/schemas/ValueExpression'
+
+ ComparisonPredicate:
+ description: >
+ A predicate that compares two value expressions. Replaces the deprecated
+ LiteralExpression that used 'term' and 'value'.
+ type: object
+ required:
+ - type
+ - left
+ - right
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["lt", "lt-eq", "gt", "gt-eq", "eq", "not-eq", "starts-with", "not-starts-with"]
+ left:
+ $ref: '#/components/schemas/ValueExpression'
+ right:
+ $ref: '#/components/schemas/ValueExpression'
+
+ SetPredicate:
+ description: >
+ A predicate that tests whether a value expression is in a set of literals.
Review Comment:
I would prefer a more neutral wording. `A predicate operates on a set of literals`.
The current wording seems to imply or favor the `in` variant,
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: Update REST spec expressions to match new expressions spec [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #17138:
URL: https://github.com/apache/iceberg/pull/17138#discussion_r3591062548
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2606,7 +2630,205 @@ components:
child:
$ref: '#/components/schemas/Expression'
+ UnaryPredicate:
+ description: >
+ A predicate that tests a single value expression.
+ type: object
+ additionalProperties: false
+ required:
+ - type
+ - child
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["is-null", "not-null", "is-nan", "not-nan"]
+ child:
+ $ref: '#/components/schemas/ValueExpression'
+
+ ComparisonPredicate:
+ description: >
+ A predicate that compares two value expressions.
+ type: object
+ additionalProperties: false
+ required:
+ - type
+ - left
+ - right
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["lt", "lt-eq", "gt", "gt-eq", "eq", "not-eq", "starts-with", "not-starts-with"]
+ left:
+ $ref: '#/components/schemas/ValueExpression'
+ right:
+ $ref: '#/components/schemas/ValueExpression'
+
+ SetPredicate:
+ description: >
+ A predicate that operates on a list of literals.
+ type: object
+ additionalProperties: false
+ required:
+ - type
+ - child
+ - values
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["in", "not-in"]
+ child:
+ $ref: '#/components/schemas/ValueExpression'
+ values:
+ $ref: '#/components/schemas/Literals'
+
+ Literal:
+ description: >
+ A literal is either a bare value, an untyped literal object, or a
+ typed literal object.
+ oneOf:
+ - $ref: '#/components/schemas/PrimitiveTypeValue'
+ - type: object
+ additionalProperties: false
+ required:
+ - type
+ - value
+ properties:
+ type:
+ type: string
+ const: "literal"
+ value:
+ $ref: '#/components/schemas/PrimitiveTypeValue'
+ data-type:
+ $ref: '#/components/schemas/PrimitiveType'
+
+ Literals:
+ description: >
+ Literals is either a bare array of literals or a typed object with
+ an explicit data-type applied to all values.
+ oneOf:
+ - type: array
+ items:
+ $ref: '#/components/schemas/Literal'
+ - type: object
+ additionalProperties: false
+ required:
+ - type
+ - values
+ - data-type
+ properties:
+ type:
+ type: string
+ const: "literals"
+ values:
+ type: array
+ items:
+ $ref: '#/components/schemas/PrimitiveTypeValue'
+ data-type:
+ $ref: '#/components/schemas/PrimitiveType'
+
+ FieldReference:
+ description: >
+ A reference to a field. Either a bound reference (by field ID) or
+ an unbound reference (by name). The context in which an expression
+ is used determines which form is valid.
+ oneOf:
+ - $ref: '#/components/schemas/FieldReferenceById'
+ - $ref: '#/components/schemas/FieldReferenceByName'
+
+ FieldReferenceById:
Review Comment:
Why not `IdReference` and `NamedReference`? These names are just a bit longer than needed. Another alternative is `UnboundReference` and `BoundReference`. But maybe you're avoiding those names because "bound" might imply its output type is known?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: Update REST spec expressions to match new expressions spec [iceberg]
Posted by "danielcweeks (via GitHub)" <gi...@apache.org>.
danielcweeks commented on code in PR #17138:
URL: https://github.com/apache/iceberg/pull/17138#discussion_r3641912660
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2528,15 +2528,16 @@ components:
items:
type: integer
- Expression:
+ Predicate:
Review Comment:
ok, based on the other comment, it seems we don't provide any guarantees for code generated based of this as long as the serialized representation for requests/responses doesn't change. I'm not sure if I love that, but it's a reasonable justification.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: Update REST spec expressions to match new expressions spec [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #17138:
URL: https://github.com/apache/iceberg/pull/17138#discussion_r3591082890
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2606,7 +2630,205 @@ components:
child:
$ref: '#/components/schemas/Expression'
+ UnaryPredicate:
+ description: >
+ A predicate that tests a single value expression.
+ type: object
+ additionalProperties: false
+ required:
+ - type
+ - child
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["is-null", "not-null", "is-nan", "not-nan"]
+ child:
+ $ref: '#/components/schemas/ValueExpression'
+
+ ComparisonPredicate:
+ description: >
+ A predicate that compares two value expressions.
+ type: object
+ additionalProperties: false
+ required:
+ - type
+ - left
+ - right
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["lt", "lt-eq", "gt", "gt-eq", "eq", "not-eq", "starts-with", "not-starts-with"]
+ left:
+ $ref: '#/components/schemas/ValueExpression'
+ right:
+ $ref: '#/components/schemas/ValueExpression'
+
+ SetPredicate:
+ description: >
+ A predicate that operates on a list of literals.
+ type: object
+ additionalProperties: false
+ required:
+ - type
+ - child
+ - values
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["in", "not-in"]
+ child:
+ $ref: '#/components/schemas/ValueExpression'
+ values:
+ $ref: '#/components/schemas/Literals'
+
+ Literal:
+ description: >
+ A literal is either a bare value, an untyped literal object, or a
+ typed literal object.
+ oneOf:
+ - $ref: '#/components/schemas/PrimitiveTypeValue'
+ - type: object
+ additionalProperties: false
+ required:
+ - type
+ - value
+ properties:
+ type:
+ type: string
+ const: "literal"
+ value:
+ $ref: '#/components/schemas/PrimitiveTypeValue'
+ data-type:
+ $ref: '#/components/schemas/PrimitiveType'
+
+ Literals:
+ description: >
+ Literals is either a bare array of literals or a typed object with
+ an explicit data-type applied to all values.
+ oneOf:
+ - type: array
+ items:
+ $ref: '#/components/schemas/Literal'
+ - type: object
+ additionalProperties: false
+ required:
+ - type
+ - values
+ - data-type
+ properties:
+ type:
+ type: string
+ const: "literals"
+ values:
+ type: array
+ items:
+ $ref: '#/components/schemas/PrimitiveTypeValue'
+ data-type:
+ $ref: '#/components/schemas/PrimitiveType'
+
+ FieldReference:
+ description: >
+ A reference to a field. Either a bound reference (by field ID) or
+ an unbound reference (by name). The context in which an expression
+ is used determines which form is valid.
+ oneOf:
+ - $ref: '#/components/schemas/FieldReferenceById'
+ - $ref: '#/components/schemas/FieldReferenceByName'
+
+ FieldReferenceById:
+ description: A bound reference to a field by field ID.
+ type: object
+ additionalProperties: false
+ required:
+ - type
+ - id
+ properties:
+ type:
+ type: string
+ const: "reference"
+ id:
+ type: integer
+
+ FieldReferenceByName:
+ description: An unbound reference to a field by name.
+ type: object
+ additionalProperties: false
+ required:
+ - type
+ - name
+ properties:
+ type:
+ type: string
+ const: "reference"
+ name:
+ type: string
+
+ Apply:
+ description: >
+ A function application on zero or more value expressions or predicates.
+ type: object
+ additionalProperties: false
+ required:
+ - type
+ - function
+ - arguments
+ properties:
+ type:
+ type: string
+ const: "apply"
+ function:
+ $ref: '#/components/schemas/FunctionReference'
+ arguments:
+ type: array
+ items:
+ anyOf:
Review Comment:
Nit: I would prefer a `FunctionArgument` type used here rather than embedding the `anyOf`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: Update REST spec expressions to match new expressions spec [iceberg]
Posted by "danielcweeks (via GitHub)" <gi...@apache.org>.
danielcweeks commented on PR #17138:
URL: https://github.com/apache/iceberg/pull/17138#issuecomment-5036459321
I'm a little concerned about how we're evolving the spec here and what impact it may have on generated code. Do we care about compatibility at that level?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: Update REST spec expressions to match new expressions spec [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #17138:
URL: https://github.com/apache/iceberg/pull/17138#discussion_r3646785176
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2604,63 +2619,216 @@ components:
$ref: '#/components/schemas/ExpressionType'
const: "not"
child:
- $ref: '#/components/schemas/Expression'
+ $ref: '#/components/schemas/Predicate'
- UnaryExpression:
+ UnaryPredicate:
+ description: >
+ A predicate that tests a single value expression. Accepts either
+ 'child' (preferred) or 'term' (deprecated) to identify the operand.
type: object
+ additionalProperties: false
required:
- type
- - term
properties:
type:
$ref: '#/components/schemas/ExpressionType'
enum: ["is-null", "not-null", "is-nan", "not-nan"]
+ child:
+ $ref: '#/components/schemas/ValueExpression'
term:
+ deprecated: true
+ description: "Deprecated. Use 'child' instead."
$ref: '#/components/schemas/Term'
- LiteralExpression:
+ ComparisonPredicate:
+ description: >
+ A predicate that compares two value expressions. Accepts either
+ 'left'/'right' (preferred) or 'term'/'value' (deprecated).
type: object
+ additionalProperties: false
required:
Review Comment:
I don't think that this needs to enforce all requirements structurally. This already states that using "left" and "right" is preferred and "term" and "value" are deprecated. If you send "left" but not "right" then that's going to be an error -- and an obvious one for both producers and consumers.
The alternative to split this into 3 types (deprecated, current, and a union) is a lot larger and doesn't help make this substantially more clear. I doubt people are going to have trouble understanding that "left" and "right" go together.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: Update REST spec expressions to match new expressions spec [iceberg]
Posted by "nevzheng (via GitHub)" <gi...@apache.org>.
nevzheng commented on code in PR #17138:
URL: https://github.com/apache/iceberg/pull/17138#discussion_r3565562451
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2606,7 +2630,206 @@ components:
child:
$ref: '#/components/schemas/Expression'
+ UnaryPredicate:
+ description: >
+ A predicate that tests a single value expression.
+ type: object
+ required:
+ - type
+ - child
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["is-null", "not-null", "is-nan", "not-nan"]
+ child:
+ $ref: '#/components/schemas/ValueExpression'
+
+ ComparisonPredicate:
+ description: >
+ A predicate that compares two value expressions.
+ type: object
+ required:
+ - type
+ - left
+ - right
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["lt", "lt-eq", "gt", "gt-eq", "eq", "not-eq", "starts-with", "not-starts-with"]
+ left:
+ $ref: '#/components/schemas/ValueExpression'
+ right:
+ $ref: '#/components/schemas/ValueExpression'
+
+ SetPredicate:
+ description: >
+ A predicate that operates on a list of literals.
+ type: object
+ required:
+ - type
+ - child
+ - values
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["in", "not-in"]
+ child:
+ $ref: '#/components/schemas/ValueExpression'
+ values:
+ $ref: '#/components/schemas/Literals'
+
+ Literal:
+ description: >
+ A literal value expression. Three JSON forms are accepted: a bare
+ scalar value, a typed literal object without a data-type, or a
+ typed literal object with an explicit data-type.
+ oneOf:
+ - type: string
+ - type: number
+ - type: boolean
Review Comment:
+1
IIUC, this should be `$ref: '#/components/schemas/PrimitiveTypeValue'`. the spec says `VALUE: single value JSON from the table spec`. And string number boolean seem limiting.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: Update REST spec expressions to match new expressions spec [iceberg]
Posted by "zhjwpku (via GitHub)" <gi...@apache.org>.
zhjwpku commented on code in PR #17138:
URL: https://github.com/apache/iceberg/pull/17138#discussion_r3649295987
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2604,63 +2619,216 @@ components:
$ref: '#/components/schemas/ExpressionType'
const: "not"
child:
- $ref: '#/components/schemas/Expression'
+ $ref: '#/components/schemas/Predicate'
- UnaryExpression:
+ UnaryPredicate:
+ description: >
+ A predicate that tests a single value expression. Accepts either
+ 'child' (preferred) or 'term' (deprecated) to identify the operand.
type: object
+ additionalProperties: false
required:
- type
- - term
properties:
type:
$ref: '#/components/schemas/ExpressionType'
enum: ["is-null", "not-null", "is-nan", "not-nan"]
+ child:
+ $ref: '#/components/schemas/ValueExpression'
term:
+ deprecated: true
+ description: "Deprecated. Use 'child' instead."
$ref: '#/components/schemas/Term'
- LiteralExpression:
+ ComparisonPredicate:
+ description: >
+ A predicate that compares two value expressions. Accepts either
+ 'left'/'right' (preferred) or 'term'/'value' (deprecated).
type: object
+ additionalProperties: false
required:
Review Comment:
Fair enough, +1 from me
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: Update REST spec expressions to match new expressions spec [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #17138:
URL: https://github.com/apache/iceberg/pull/17138#discussion_r3591128287
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2646,20 +2878,37 @@ components:
term:
$ref: '#/components/schemas/Term'
values:
- type: array
- items:
- $ref: '#/components/schemas/PrimitiveTypeValue'
+ $ref: '#/components/schemas/Literals'
Term:
oneOf:
- $ref: '#/components/schemas/Reference'
- $ref: '#/components/schemas/TransformTerm'
+ - $ref: '#/components/schemas/DeprecatedReference'
Reference:
type: string
example:
- "column-name"
+ DeprecatedReference:
Review Comment:
This is interesting because it is apparently not already allowed. `Reference` has `type: string`, even though the Java implementation [supports deserializing the object form](https://github.com/apache/iceberg/blob/main/core/src/main/java/org/apache/iceberg/expressions/ExpressionParser.java#L411). Looks like it [only produces the string form](https://github.com/apache/iceberg/blob/main/core/src/main/java/org/apache/iceberg/expressions/ExpressionParser.java#L246).
As a result, I think we have the option of saying that this was never allowed and not adding it to this spec now, even though the Java implementation would support it. Or, we can try to standardize to the deprecated definition in the expressions spec and add this.
If we add this, I'd prefer to combine it with `Reference` like you did in `FunctionReference`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: Update REST spec expressions to match new expressions spec [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #17138:
URL: https://github.com/apache/iceberg/pull/17138#discussion_r3590939351
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2561,7 +2565,22 @@ components:
- "is-nan"
- "not-nan"
+ ValueExpression:
+ description: >
+ A value expression per Iceberg Expressions spec, Appendix B: a literal,
+ a field reference, or a function application. The result type is
+ determined when the expression is bound to an input schema.
Review Comment:
I don't think we need to restate parts of the spec here. This is about the structure and not about how binding or type alignment occurs. I'd omit the result type part of this since it doesn't matter to the structure.
Also, I don't think that this needs to reference Appendix B. That's a reference for converting to/from JSON but this is its own spec definition. I think this is more of a reference to the main body of the spec. Either way, they say roughly the same thing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: Update REST spec expressions to match new expressions spec [iceberg]
Posted by "nevzheng (via GitHub)" <gi...@apache.org>.
nevzheng commented on code in PR #17138:
URL: https://github.com/apache/iceberg/pull/17138#discussion_r3555555393
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2606,7 +2630,194 @@ components:
child:
$ref: '#/components/schemas/Expression'
+ UnaryPredicate:
+ description: >
+ A predicate that tests a single value expression. Replaces the deprecated
+ UnaryExpression that used 'term'.
+ type: object
+ required:
+ - type
+ - child
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["is-null", "not-null", "is-nan", "not-nan"]
+ child:
+ $ref: '#/components/schemas/ValueExpression'
+
+ ComparisonPredicate:
+ description: >
+ A predicate that compares two value expressions. Replaces the deprecated
+ LiteralExpression that used 'term' and 'value'.
+ type: object
+ required:
+ - type
+ - left
+ - right
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["lt", "lt-eq", "gt", "gt-eq", "eq", "not-eq", "starts-with", "not-starts-with"]
+ left:
+ $ref: '#/components/schemas/ValueExpression'
+ right:
+ $ref: '#/components/schemas/ValueExpression'
+
+ SetPredicate:
+ description: >
+ A predicate that tests whether a value expression is in a set of literals.
+ Replaces the deprecated SetExpression that used 'term'.
+ type: object
+ required:
+ - type
+ - child
+ - values
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["in", "not-in"]
+ child:
+ $ref: '#/components/schemas/ValueExpression'
+ values:
+ $ref: '#/components/schemas/Literals'
+
+ Literal:
+ description: >
+ A literal value expression per Iceberg Expressions spec, Appendix B.
+ Three JSON forms are accepted: a bare scalar value, a typed literal
+ object without a data-type, or a typed literal object with an
+ explicit data-type.
+ oneOf:
+ - type: string
+ - type: number
+ - type: integer
+ - type: boolean
+ - type: 'null'
+ - type: object
+ required:
+ - type
+ - value
+ properties:
+ type:
+ type: string
+ const: "literal"
+ value:
+ $ref: '#/components/schemas/PrimitiveTypeValue'
+ data-type:
+ $ref: '#/components/schemas/PrimitiveType'
+
+ Literals:
+ description: >
+ A set of literal values per Iceberg Expressions spec, Appendix B.
+ Two JSON forms: a bare array of literals, or a typed object with an
+ explicit data-type applied to all values.
+ oneOf:
+ - type: array
+ items:
+ $ref: '#/components/schemas/Literal'
+ - type: object
+ required:
+ - type
+ - values
+ properties:
+ type:
+ type: string
+ const: "literals"
+ values:
+ type: array
+ items:
+ $ref: '#/components/schemas/PrimitiveTypeValue'
+ data-type:
+ $ref: '#/components/schemas/PrimitiveType'
+
+ FieldReference:
Review Comment:
Should we have two types? We're allowed to define our own. `FieldReferenceByName`, `FieldReferenceById`, `FieldReference (FieldReferenceByName| FieldReferenceById). This allows expressions to specify which variant they accept.
```openapi
FieldReference:
description: >
A reference to a field, by field ID or by name.
oneOf:
- $ref: '#/components/schemas/FieldReferenceById'
- $ref: '#/components/schemas/FieldReferenceByName'
FieldReferenceById:
description: A reference to a field by field ID (bound).
type: object
required:
- type
- id
properties:
type:
type: string
const: "reference"
id:
type: integer
FieldReferenceByName:
description: A reference to a field by name (unbound).
type: object
required:
- type
- name
properties:
type:
type: string
const: "reference"
name:
type: string
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: Update REST spec expressions to match new expressions spec [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #17138:
URL: https://github.com/apache/iceberg/pull/17138#discussion_r3591029664
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2606,7 +2630,205 @@ components:
child:
$ref: '#/components/schemas/Expression'
+ UnaryPredicate:
+ description: >
+ A predicate that tests a single value expression.
+ type: object
+ additionalProperties: false
+ required:
+ - type
+ - child
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["is-null", "not-null", "is-nan", "not-nan"]
+ child:
+ $ref: '#/components/schemas/ValueExpression'
+
+ ComparisonPredicate:
+ description: >
+ A predicate that compares two value expressions.
+ type: object
+ additionalProperties: false
+ required:
+ - type
+ - left
+ - right
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["lt", "lt-eq", "gt", "gt-eq", "eq", "not-eq", "starts-with", "not-starts-with"]
+ left:
+ $ref: '#/components/schemas/ValueExpression'
+ right:
+ $ref: '#/components/schemas/ValueExpression'
+
+ SetPredicate:
Review Comment:
Is it possible to combine this with the now-deprecated `SetExpression` to avoid more changes?
The main difference is that this uses `child` instead of `term`. But this could include `term` as a field with documentation that it should not be produced rather than replacing and deprecating all of `SetExpression`.
Similarly, `SetExpression` had a `values` field that is now of type `Literals`. `Literals` could be an array of `Literal` and `Literal` could be a `PrimitiveTypeValue`. That means the `Literals` definition used here is a superset of what was possible before. If we can solve the problem of allowing `term` here then we don't need to have two (this one and a deprecated version). We can just have this one.
The last issue is that naming may be a problem, but I think that we could also continue using `SetExpression` if it isn't deprecated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: Update REST spec expressions to match new expressions spec [iceberg]
Posted by "nevzheng (via GitHub)" <gi...@apache.org>.
nevzheng commented on code in PR #17138:
URL: https://github.com/apache/iceberg/pull/17138#discussion_r3555623977
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2606,7 +2630,194 @@ components:
child:
$ref: '#/components/schemas/Expression'
+ UnaryPredicate:
+ description: >
+ A predicate that tests a single value expression. Replaces the deprecated
+ UnaryExpression that used 'term'.
+ type: object
+ required:
+ - type
+ - child
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["is-null", "not-null", "is-nan", "not-nan"]
+ child:
+ $ref: '#/components/schemas/ValueExpression'
+
+ ComparisonPredicate:
+ description: >
+ A predicate that compares two value expressions. Replaces the deprecated
+ LiteralExpression that used 'term' and 'value'.
+ type: object
+ required:
+ - type
+ - left
+ - right
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["lt", "lt-eq", "gt", "gt-eq", "eq", "not-eq", "starts-with", "not-starts-with"]
+ left:
+ $ref: '#/components/schemas/ValueExpression'
+ right:
+ $ref: '#/components/schemas/ValueExpression'
+
+ SetPredicate:
+ description: >
+ A predicate that tests whether a value expression is in a set of literals.
+ Replaces the deprecated SetExpression that used 'term'.
+ type: object
+ required:
+ - type
+ - child
+ - values
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["in", "not-in"]
+ child:
+ $ref: '#/components/schemas/ValueExpression'
+ values:
+ $ref: '#/components/schemas/Literals'
+
+ Literal:
+ description: >
+ A literal value expression per Iceberg Expressions spec, Appendix B.
+ Three JSON forms are accepted: a bare scalar value, a typed literal
+ object without a data-type, or a typed literal object with an
+ explicit data-type.
+ oneOf:
+ - type: string
+ - type: number
+ - type: integer
+ - type: boolean
+ - type: 'null'
+ - type: object
+ required:
+ - type
+ - value
+ properties:
+ type:
+ type: string
+ const: "literal"
+ value:
+ $ref: '#/components/schemas/PrimitiveTypeValue'
+ data-type:
+ $ref: '#/components/schemas/PrimitiveType'
+
+ Literals:
+ description: >
+ A set of literal values per Iceberg Expressions spec, Appendix B.
+ Two JSON forms: a bare array of literals, or a typed object with an
+ explicit data-type applied to all values.
+ oneOf:
+ - type: array
+ items:
+ $ref: '#/components/schemas/Literal'
+ - type: object
+ required:
+ - type
+ - values
+ properties:
+ type:
+ type: string
+ const: "literals"
+ values:
+ type: array
+ items:
+ $ref: '#/components/schemas/PrimitiveTypeValue'
+ data-type:
+ $ref: '#/components/schemas/PrimitiveType'
+
+ FieldReference:
+ description: >
+ A reference to a field per Iceberg Expressions spec, Appendix B.
+ Either a bound reference (by field ID) or an unbound reference (by
+ name). The context in which an expression is used determines which
+ form is valid.
+ oneOf:
+ - type: object
+ required:
+ - type
+ - id
+ properties:
+ type:
+ type: string
+ const: "reference"
+ id:
+ type: integer
+ - type: object
+ required:
+ - type
+ - name
+ properties:
+ type:
+ type: string
+ const: "reference"
+ name:
+ type: string
+
+ Apply:
+ description: >
+ A function application per Iceberg Expressions spec, Appendix B.
+ Represents the result of calling a function on zero or more value
+ expressions or predicates.
+ type: object
+ required:
+ - type
+ - function
+ - arguments
+ properties:
+ type:
+ type: string
+ const: "apply"
+ function:
+ $ref: '#/components/schemas/FunctionReference'
+ arguments:
Review Comment:
LGTM. withdrawn. this looks correct.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: Update REST spec expressions to match new expressions spec [iceberg]
Posted by "danielcweeks (via GitHub)" <gi...@apache.org>.
danielcweeks commented on code in PR #17138:
URL: https://github.com/apache/iceberg/pull/17138#discussion_r3623927113
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2528,15 +2528,16 @@ components:
items:
type: integer
- Expression:
+ Predicate:
Review Comment:
Shouldn't we be updating this to include new representation without removing the old ones? The evolution here seems concerning.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: Update REST spec expressions to match new expressions spec [iceberg]
Posted by "singhpk234 (via GitHub)" <gi...@apache.org>.
singhpk234 commented on code in PR #17138:
URL: https://github.com/apache/iceberg/pull/17138#discussion_r3624868777
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2528,15 +2528,16 @@ components:
items:
type: integer
- Expression:
+ Predicate:
Review Comment:
the new one contains both old and new representation here, please check this
https://github.com/apache/iceberg/pull/17138/changes#diff-02549ca620d020dc9ead80088cc14e311e12a69651fa8d394cd41a4308debb2eR2624
so it captures both old and new
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: Update REST spec expressions to match new expressions spec [iceberg]
Posted by "zhjwpku (via GitHub)" <gi...@apache.org>.
zhjwpku commented on code in PR #17138:
URL: https://github.com/apache/iceberg/pull/17138#discussion_r3642945198
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2604,63 +2619,216 @@ components:
$ref: '#/components/schemas/ExpressionType'
const: "not"
child:
- $ref: '#/components/schemas/Expression'
+ $ref: '#/components/schemas/Predicate'
- UnaryExpression:
+ UnaryPredicate:
+ description: >
+ A predicate that tests a single value expression. Accepts either
+ 'child' (preferred) or 'term' (deprecated) to identify the operand.
type: object
+ additionalProperties: false
required:
- type
- - term
properties:
type:
$ref: '#/components/schemas/ExpressionType'
enum: ["is-null", "not-null", "is-nan", "not-nan"]
+ child:
+ $ref: '#/components/schemas/ValueExpression'
term:
+ deprecated: true
+ description: "Deprecated. Use 'child' instead."
$ref: '#/components/schemas/Term'
- LiteralExpression:
+ ComparisonPredicate:
+ description: >
+ A predicate that compares two value expressions. Accepts either
+ 'left'/'right' (preferred) or 'term'/'value' (deprecated).
type: object
+ additionalProperties: false
required:
Review Comment:
This currently allows predicates with missing operands, for example:
```json
{"type": "eq"}
```
It also allows partially specified or mixed representations, such as `left` with `term/value`.
```
{
"type": "eq",
"left": {"type": "reference", "name": "a"},
"term": "b",
"value": 1
}
```
According to the expressions spec, the current form must contain both `left` and `right`, while the deprecated form must contain both `term` and `value`. Could we model the current and deprecated representations as separate schemas and combine them with `oneOf`?
The same issue applies to `UnaryPredicate` and `SetPredicate`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: Update REST spec expressions to match new expressions spec [iceberg]
Posted by "singhpk234 (via GitHub)" <gi...@apache.org>.
singhpk234 commented on code in PR #17138:
URL: https://github.com/apache/iceberg/pull/17138#discussion_r3646786388
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2604,63 +2619,216 @@ components:
$ref: '#/components/schemas/ExpressionType'
const: "not"
child:
- $ref: '#/components/schemas/Expression'
+ $ref: '#/components/schemas/Predicate'
- UnaryExpression:
+ UnaryPredicate:
+ description: >
+ A predicate that tests a single value expression. Accepts either
+ 'child' (preferred) or 'term' (deprecated) to identify the operand.
type: object
+ additionalProperties: false
required:
- type
- - term
properties:
type:
$ref: '#/components/schemas/ExpressionType'
enum: ["is-null", "not-null", "is-nan", "not-nan"]
+ child:
+ $ref: '#/components/schemas/ValueExpression'
term:
+ deprecated: true
+ description: "Deprecated. Use 'child' instead."
$ref: '#/components/schemas/Term'
- LiteralExpression:
+ ComparisonPredicate:
+ description: >
+ A predicate that compares two value expressions. Accepts either
+ 'left'/'right' (preferred) or 'term'/'value' (deprecated).
type: object
+ additionalProperties: false
required:
Review Comment:
Please check this discussion : https://github.com/apache/iceberg/pull/17138#issuecomment-5063288093
the spec generated code can produce this half backed structure but spec text forbids it, and the concencus on this is to not rely on generated api from spec but use this to create implementation like we do in java api
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: Update REST spec expressions to match new expressions spec [iceberg]
Posted by "amogh-jahagirdar (via GitHub)" <gi...@apache.org>.
amogh-jahagirdar commented on code in PR #17138:
URL: https://github.com/apache/iceberg/pull/17138#discussion_r3650447153
##########
open-api/rest-catalog-open-api.py:
##########
@@ -152,6 +152,11 @@ class ExpressionType(RootModel[str]):
class TrueExpression(BaseModel):
+ """
+ Deprecated. Use the bare boolean literal `true` as a predicate instead.
Review Comment:
Very minor: Not sure the adjective "bare" is really doing anything here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: Update REST spec expressions to match new expressions spec [iceberg]
Posted by "singhpk234 (via GitHub)" <gi...@apache.org>.
singhpk234 commented on PR #17138:
URL: https://github.com/apache/iceberg/pull/17138#issuecomment-5037847597
> I'm a little concerned about how we're evolving the spec here and what impact it may have on generated code. Do we care about compatibility at that level?
I had same concern too, the generated code will not be correct but the reality we don't rely on generated code at all atleast in Java spec hence i moved to suggestion Ryan had since this simplifies things a lot ...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: Update REST spec expressions to match new expressions spec [iceberg]
Posted by "nevzheng (via GitHub)" <gi...@apache.org>.
nevzheng commented on code in PR #17138:
URL: https://github.com/apache/iceberg/pull/17138#discussion_r3555315607
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2561,7 +2565,22 @@ components:
- "is-nan"
- "not-nan"
+ ValueExpression:
+ description: >
+ A value expression per Iceberg Expressions spec, Appendix B: a literal,
Review Comment:
How about `A value is a literal, field reference, or function application. See Iceberg Expressions spec, Appendix B`. I think the result type part doesn't add anything.
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2530,13 +2530,17 @@ components:
Expression:
oneOf:
+ - type: boolean
Review Comment:
What if we did something like this. I'm thinking we wouldn't have to mention "Iceberg Expression Spec in description this way" It seems a bit verbose.
```txt
// deprecated.
...list deprecated
// Iceberg Expression Spec, Appendix B
... list current
```
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2606,7 +2630,194 @@ components:
child:
$ref: '#/components/schemas/Expression'
+ UnaryPredicate:
+ description: >
+ A predicate that tests a single value expression. Replaces the deprecated
+ UnaryExpression that used 'term'.
+ type: object
+ required:
+ - type
+ - child
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["is-null", "not-null", "is-nan", "not-nan"]
+ child:
+ $ref: '#/components/schemas/ValueExpression'
+
+ ComparisonPredicate:
+ description: >
+ A predicate that compares two value expressions. Replaces the deprecated
+ LiteralExpression that used 'term' and 'value'.
+ type: object
+ required:
+ - type
+ - left
+ - right
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["lt", "lt-eq", "gt", "gt-eq", "eq", "not-eq", "starts-with", "not-starts-with"]
+ left:
+ $ref: '#/components/schemas/ValueExpression'
+ right:
+ $ref: '#/components/schemas/ValueExpression'
+
+ SetPredicate:
+ description: >
+ A predicate that tests whether a value expression is in a set of literals.
+ Replaces the deprecated SetExpression that used 'term'.
+ type: object
+ required:
+ - type
+ - child
+ - values
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["in", "not-in"]
+ child:
+ $ref: '#/components/schemas/ValueExpression'
+ values:
+ $ref: '#/components/schemas/Literals'
+
+ Literal:
+ description: >
+ A literal value expression per Iceberg Expressions spec, Appendix B.
+ Three JSON forms are accepted: a bare scalar value, a typed literal
+ object without a data-type, or a typed literal object with an
+ explicit data-type.
+ oneOf:
+ - type: string
+ - type: number
+ - type: integer
+ - type: boolean
+ - type: 'null'
+ - type: object
+ required:
+ - type
+ - value
+ properties:
+ type:
+ type: string
+ const: "literal"
+ value:
+ $ref: '#/components/schemas/PrimitiveTypeValue'
+ data-type:
+ $ref: '#/components/schemas/PrimitiveType'
+
+ Literals:
+ description: >
+ A set of literal values per Iceberg Expressions spec, Appendix B.
+ Two JSON forms: a bare array of literals, or a typed object with an
+ explicit data-type applied to all values.
+ oneOf:
+ - type: array
+ items:
+ $ref: '#/components/schemas/Literal'
+ - type: object
+ required:
+ - type
+ - values
+ properties:
+ type:
+ type: string
+ const: "literals"
+ values:
+ type: array
+ items:
+ $ref: '#/components/schemas/PrimitiveTypeValue'
+ data-type:
+ $ref: '#/components/schemas/PrimitiveType'
+
+ FieldReference:
+ description: >
+ A reference to a field per Iceberg Expressions spec, Appendix B.
+ Either a bound reference (by field ID) or an unbound reference (by
+ name). The context in which an expression is used determines which
+ form is valid.
+ oneOf:
+ - type: object
+ required:
+ - type
+ - id
+ properties:
+ type:
+ type: string
+ const: "reference"
+ id:
+ type: integer
+ - type: object
+ required:
+ - type
+ - name
+ properties:
+ type:
+ type: string
+ const: "reference"
+ name:
+ type: string
+
+ Apply:
+ description: >
+ A function application per Iceberg Expressions spec, Appendix B.
+ Represents the result of calling a function on zero or more value
+ expressions or predicates.
+ type: object
+ required:
+ - type
+ - function
+ - arguments
+ properties:
+ type:
+ type: string
+ const: "apply"
+ function:
+ $ref: '#/components/schemas/FunctionReference'
+ arguments:
Review Comment:
I think we could be missing variants.
Arguments is a list of function arg. And Function arg is one of 4 things. and we only have 2 here.
```
APPLY: { "type": "apply", "function": FUNC_REF, "arguments": [ FUNC_ARG* ] }
FUNC_ARG: EXPR | PREDICATE
FUNC_REF: NAME
| [ NAME* ]
| { "identifier": [ NAME* ] }
| { "catalog": NAME, "identifier": [ NAME* ] }
```
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2606,7 +2630,194 @@ components:
child:
$ref: '#/components/schemas/Expression'
+ UnaryPredicate:
+ description: >
+ A predicate that tests a single value expression. Replaces the deprecated
+ UnaryExpression that used 'term'.
+ type: object
+ required:
+ - type
+ - child
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["is-null", "not-null", "is-nan", "not-nan"]
+ child:
+ $ref: '#/components/schemas/ValueExpression'
+
+ ComparisonPredicate:
+ description: >
+ A predicate that compares two value expressions. Replaces the deprecated
+ LiteralExpression that used 'term' and 'value'.
+ type: object
+ required:
+ - type
+ - left
+ - right
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["lt", "lt-eq", "gt", "gt-eq", "eq", "not-eq", "starts-with", "not-starts-with"]
+ left:
+ $ref: '#/components/schemas/ValueExpression'
+ right:
+ $ref: '#/components/schemas/ValueExpression'
+
+ SetPredicate:
+ description: >
+ A predicate that tests whether a value expression is in a set of literals.
+ Replaces the deprecated SetExpression that used 'term'.
+ type: object
+ required:
+ - type
+ - child
+ - values
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["in", "not-in"]
+ child:
+ $ref: '#/components/schemas/ValueExpression'
+ values:
+ $ref: '#/components/schemas/Literals'
+
+ Literal:
+ description: >
+ A literal value expression per Iceberg Expressions spec, Appendix B.
+ Three JSON forms are accepted: a bare scalar value, a typed literal
+ object without a data-type, or a typed literal object with an
+ explicit data-type.
+ oneOf:
+ - type: string
+ - type: number
+ - type: integer
+ - type: boolean
+ - type: 'null'
+ - type: object
+ required:
+ - type
+ - value
+ properties:
+ type:
+ type: string
+ const: "literal"
+ value:
+ $ref: '#/components/schemas/PrimitiveTypeValue'
+ data-type:
+ $ref: '#/components/schemas/PrimitiveType'
+
+ Literals:
+ description: >
+ A set of literal values per Iceberg Expressions spec, Appendix B.
+ Two JSON forms: a bare array of literals, or a typed object with an
+ explicit data-type applied to all values.
+ oneOf:
+ - type: array
+ items:
+ $ref: '#/components/schemas/Literal'
+ - type: object
+ required:
+ - type
+ - values
+ properties:
+ type:
+ type: string
+ const: "literals"
+ values:
+ type: array
+ items:
+ $ref: '#/components/schemas/PrimitiveTypeValue'
+ data-type:
+ $ref: '#/components/schemas/PrimitiveType'
+
+ FieldReference:
Review Comment:
Should we have two types? We're allowed to define our own. `FieldReferenceByName`, `FieldReferenceById`, `FieldReference (FieldReferenceByName| FieldReferenceById). This allows expressions to specify which variant they accept.
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2606,7 +2630,194 @@ components:
child:
$ref: '#/components/schemas/Expression'
+ UnaryPredicate:
+ description: >
+ A predicate that tests a single value expression. Replaces the deprecated
Review Comment:
I would recommend NOT referring to past version in current code. It seems unnecessary.
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2606,7 +2630,194 @@ components:
child:
$ref: '#/components/schemas/Expression'
+ UnaryPredicate:
Review Comment:
I'm not sure this follows the spec. For example, we don't have `Predicate= true | false | UnaryOp | CmpOp |.... `
We flatten by a level, and define `UnaryPredidicate`, `SetPredicate`, `ComparisonPredicate`...
Is that acceptable for spec compliance? I would defer to Ryan.
https://github.com/apache/iceberg/blob/main/format/expressions-spec.md
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2606,7 +2630,194 @@ components:
child:
$ref: '#/components/schemas/Expression'
+ UnaryPredicate:
+ description: >
+ A predicate that tests a single value expression. Replaces the deprecated
+ UnaryExpression that used 'term'.
+ type: object
+ required:
+ - type
+ - child
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["is-null", "not-null", "is-nan", "not-nan"]
+ child:
+ $ref: '#/components/schemas/ValueExpression'
+
+ ComparisonPredicate:
+ description: >
+ A predicate that compares two value expressions. Replaces the deprecated
+ LiteralExpression that used 'term' and 'value'.
+ type: object
+ required:
+ - type
+ - left
+ - right
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["lt", "lt-eq", "gt", "gt-eq", "eq", "not-eq", "starts-with", "not-starts-with"]
+ left:
+ $ref: '#/components/schemas/ValueExpression'
+ right:
+ $ref: '#/components/schemas/ValueExpression'
+
+ SetPredicate:
+ description: >
+ A predicate that tests whether a value expression is in a set of literals.
+ Replaces the deprecated SetExpression that used 'term'.
+ type: object
+ required:
+ - type
+ - child
+ - values
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["in", "not-in"]
+ child:
+ $ref: '#/components/schemas/ValueExpression'
+ values:
+ $ref: '#/components/schemas/Literals'
+
+ Literal:
+ description: >
+ A literal value expression per Iceberg Expressions spec, Appendix B.
+ Three JSON forms are accepted: a bare scalar value, a typed literal
+ object without a data-type, or a typed literal object with an
+ explicit data-type.
+ oneOf:
+ - type: string
+ - type: number
+ - type: integer
+ - type: boolean
+ - type: 'null'
+ - type: object
+ required:
+ - type
+ - value
+ properties:
+ type:
+ type: string
+ const: "literal"
+ value:
+ $ref: '#/components/schemas/PrimitiveTypeValue'
+ data-type:
+ $ref: '#/components/schemas/PrimitiveType'
+
+ Literals:
+ description: >
+ A set of literal values per Iceberg Expressions spec, Appendix B.
+ Two JSON forms: a bare array of literals, or a typed object with an
+ explicit data-type applied to all values.
+ oneOf:
+ - type: array
+ items:
+ $ref: '#/components/schemas/Literal'
+ - type: object
+ required:
Review Comment:
the object form of `Literals` has to have a "data type"
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2606,7 +2630,194 @@ components:
child:
$ref: '#/components/schemas/Expression'
+ UnaryPredicate:
+ description: >
+ A predicate that tests a single value expression. Replaces the deprecated
+ UnaryExpression that used 'term'.
+ type: object
+ required:
+ - type
+ - child
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["is-null", "not-null", "is-nan", "not-nan"]
+ child:
+ $ref: '#/components/schemas/ValueExpression'
+
+ ComparisonPredicate:
+ description: >
+ A predicate that compares two value expressions. Replaces the deprecated
+ LiteralExpression that used 'term' and 'value'.
+ type: object
+ required:
+ - type
+ - left
+ - right
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["lt", "lt-eq", "gt", "gt-eq", "eq", "not-eq", "starts-with", "not-starts-with"]
+ left:
+ $ref: '#/components/schemas/ValueExpression'
+ right:
+ $ref: '#/components/schemas/ValueExpression'
+
+ SetPredicate:
+ description: >
+ A predicate that tests whether a value expression is in a set of literals.
Review Comment:
I would say `A predicate operates on a set of literals`. what value you return depends on the specific implementation.
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2606,7 +2630,194 @@ components:
child:
$ref: '#/components/schemas/Expression'
+ UnaryPredicate:
+ description: >
+ A predicate that tests a single value expression. Replaces the deprecated
+ UnaryExpression that used 'term'.
+ type: object
+ required:
+ - type
+ - child
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["is-null", "not-null", "is-nan", "not-nan"]
+ child:
+ $ref: '#/components/schemas/ValueExpression'
+
+ ComparisonPredicate:
+ description: >
+ A predicate that compares two value expressions. Replaces the deprecated
+ LiteralExpression that used 'term' and 'value'.
+ type: object
+ required:
+ - type
+ - left
+ - right
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["lt", "lt-eq", "gt", "gt-eq", "eq", "not-eq", "starts-with", "not-starts-with"]
+ left:
+ $ref: '#/components/schemas/ValueExpression'
+ right:
+ $ref: '#/components/schemas/ValueExpression'
+
+ SetPredicate:
+ description: >
+ A predicate that tests whether a value expression is in a set of literals.
+ Replaces the deprecated SetExpression that used 'term'.
+ type: object
+ required:
+ - type
+ - child
+ - values
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["in", "not-in"]
+ child:
+ $ref: '#/components/schemas/ValueExpression'
+ values:
+ $ref: '#/components/schemas/Literals'
+
+ Literal:
+ description: >
+ A literal value expression per Iceberg Expressions spec, Appendix B.
+ Three JSON forms are accepted: a bare scalar value, a typed literal
+ object without a data-type, or a typed literal object with an
+ explicit data-type.
+ oneOf:
+ - type: string
+ - type: number
+ - type: integer
+ - type: boolean
+ - type: 'null'
+ - type: object
+ required:
+ - type
+ - value
+ properties:
+ type:
+ type: string
+ const: "literal"
+ value:
+ $ref: '#/components/schemas/PrimitiveTypeValue'
+ data-type:
+ $ref: '#/components/schemas/PrimitiveType'
+
+ Literals:
+ description: >
+ A set of literal values per Iceberg Expressions spec, Appendix B.
+ Two JSON forms: a bare array of literals, or a typed object with an
Review Comment:
This seems non compliant. Here's my reading of the spec.
```txt
LITERAL:
VALUE (X)
| { "type": "literal", "value": VALUE } (Y)
| { "type": "literal", "value": VALUE, "data-type": DATA_TYPE } (Z)
LITERALS:
// A LIST of 0 or More `Literal`. This is recursive. And you would end up with a list any permutation of (X), (Y), (Z) definitions
[ LITERAL* ]
// A structure capturing VALUEs of the same `DATA_TYPE`.
| { "type": "literals", "values": [ VALUE* ], "data-type": DATA_TYPE }
```
- Its not a set (non ordered, unique collection))
I would just let the recursive definition breathe.
```txt
`Literals is an array of `Literal` or, an object with an array of Value of the same type, and a data type describing the type.
```
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2606,7 +2630,194 @@ components:
child:
$ref: '#/components/schemas/Expression'
+ UnaryPredicate:
+ description: >
+ A predicate that tests a single value expression. Replaces the deprecated
+ UnaryExpression that used 'term'.
+ type: object
+ required:
+ - type
+ - child
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["is-null", "not-null", "is-nan", "not-nan"]
+ child:
+ $ref: '#/components/schemas/ValueExpression'
+
+ ComparisonPredicate:
+ description: >
+ A predicate that compares two value expressions. Replaces the deprecated
+ LiteralExpression that used 'term' and 'value'.
+ type: object
+ required:
+ - type
+ - left
+ - right
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["lt", "lt-eq", "gt", "gt-eq", "eq", "not-eq", "starts-with", "not-starts-with"]
+ left:
+ $ref: '#/components/schemas/ValueExpression'
+ right:
+ $ref: '#/components/schemas/ValueExpression'
+
+ SetPredicate:
+ description: >
+ A predicate that tests whether a value expression is in a set of literals.
+ Replaces the deprecated SetExpression that used 'term'.
+ type: object
+ required:
+ - type
+ - child
+ - values
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["in", "not-in"]
+ child:
+ $ref: '#/components/schemas/ValueExpression'
+ values:
+ $ref: '#/components/schemas/Literals'
+
+ Literal:
+ description: >
+ A literal value expression per Iceberg Expressions spec, Appendix B.
+ Three JSON forms are accepted: a bare scalar value, a typed literal
+ object without a data-type, or a typed literal object with an
+ explicit data-type.
+ oneOf:
+ - type: string
Review Comment:
can we pack the `VALUE` into a structure to make it look more like the spec. I think this is probably ok.
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2606,7 +2630,194 @@ components:
child:
$ref: '#/components/schemas/Expression'
+ UnaryPredicate:
+ description: >
+ A predicate that tests a single value expression. Replaces the deprecated
+ UnaryExpression that used 'term'.
+ type: object
+ required:
+ - type
+ - child
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["is-null", "not-null", "is-nan", "not-nan"]
+ child:
+ $ref: '#/components/schemas/ValueExpression'
+
+ ComparisonPredicate:
+ description: >
+ A predicate that compares two value expressions. Replaces the deprecated
+ LiteralExpression that used 'term' and 'value'.
+ type: object
+ required:
+ - type
+ - left
+ - right
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["lt", "lt-eq", "gt", "gt-eq", "eq", "not-eq", "starts-with", "not-starts-with"]
+ left:
+ $ref: '#/components/schemas/ValueExpression'
+ right:
+ $ref: '#/components/schemas/ValueExpression'
+
+ SetPredicate:
+ description: >
+ A predicate that tests whether a value expression is in a set of literals.
+ Replaces the deprecated SetExpression that used 'term'.
+ type: object
+ required:
+ - type
+ - child
+ - values
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["in", "not-in"]
+ child:
+ $ref: '#/components/schemas/ValueExpression'
+ values:
+ $ref: '#/components/schemas/Literals'
+
+ Literal:
+ description: >
+ A literal value expression per Iceberg Expressions spec, Appendix B.
+ Three JSON forms are accepted: a bare scalar value, a typed literal
+ object without a data-type, or a typed literal object with an
+ explicit data-type.
+ oneOf:
+ - type: string
+ - type: number
+ - type: integer
+ - type: boolean
+ - type: 'null'
+ - type: object
+ required:
+ - type
+ - value
+ properties:
+ type:
+ type: string
+ const: "literal"
+ value:
+ $ref: '#/components/schemas/PrimitiveTypeValue'
+ data-type:
+ $ref: '#/components/schemas/PrimitiveType'
+
+ Literals:
+ description: >
+ A set of literal values per Iceberg Expressions spec, Appendix B.
+ Two JSON forms: a bare array of literals, or a typed object with an
+ explicit data-type applied to all values.
+ oneOf:
+ - type: array
+ items:
+ $ref: '#/components/schemas/Literal'
+ - type: object
+ required:
+ - type
+ - values
+ properties:
+ type:
+ type: string
+ const: "literals"
+ values:
+ type: array
+ items:
+ $ref: '#/components/schemas/PrimitiveTypeValue'
+ data-type:
+ $ref: '#/components/schemas/PrimitiveType'
+
+ FieldReference:
+ description: >
+ A reference to a field per Iceberg Expressions spec, Appendix B.
+ Either a bound reference (by field ID) or an unbound reference (by
Review Comment:
I would suggest
```txt
A reference to a field by id or name.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: Update REST spec expressions to match new expressions spec [iceberg]
Posted by "nevzheng (via GitHub)" <gi...@apache.org>.
nevzheng commented on code in PR #17138:
URL: https://github.com/apache/iceberg/pull/17138#discussion_r3562700864
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2707,9 +2703,8 @@ components:
Literals:
description: >
- A set of literal values per Iceberg Expressions spec, Appendix B.
- Two JSON forms: a bare array of literals, or a typed object with an
- explicit data-type applied to all values.
+ A list of literal values. Two JSON forms: a bare array of literals,
+ or a typed object with an explicit data-type applied to all values.
Review Comment:
A literals value is either a bare array of literals or a typed object with an explicit data-type applied to all values.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: Update REST spec expressions to match new expressions spec [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #17138:
URL: https://github.com/apache/iceberg/pull/17138#discussion_r3591090020
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2606,7 +2630,205 @@ components:
child:
$ref: '#/components/schemas/Expression'
+ UnaryPredicate:
+ description: >
+ A predicate that tests a single value expression.
+ type: object
+ additionalProperties: false
+ required:
+ - type
+ - child
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["is-null", "not-null", "is-nan", "not-nan"]
+ child:
+ $ref: '#/components/schemas/ValueExpression'
+
+ ComparisonPredicate:
+ description: >
+ A predicate that compares two value expressions.
+ type: object
+ additionalProperties: false
+ required:
+ - type
+ - left
+ - right
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["lt", "lt-eq", "gt", "gt-eq", "eq", "not-eq", "starts-with", "not-starts-with"]
+ left:
+ $ref: '#/components/schemas/ValueExpression'
+ right:
+ $ref: '#/components/schemas/ValueExpression'
+
+ SetPredicate:
+ description: >
+ A predicate that operates on a list of literals.
+ type: object
+ additionalProperties: false
+ required:
+ - type
+ - child
+ - values
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["in", "not-in"]
+ child:
+ $ref: '#/components/schemas/ValueExpression'
+ values:
+ $ref: '#/components/schemas/Literals'
+
+ Literal:
+ description: >
+ A literal is either a bare value, an untyped literal object, or a
+ typed literal object.
+ oneOf:
+ - $ref: '#/components/schemas/PrimitiveTypeValue'
+ - type: object
+ additionalProperties: false
+ required:
+ - type
+ - value
+ properties:
+ type:
+ type: string
+ const: "literal"
+ value:
+ $ref: '#/components/schemas/PrimitiveTypeValue'
+ data-type:
+ $ref: '#/components/schemas/PrimitiveType'
+
+ Literals:
+ description: >
+ Literals is either a bare array of literals or a typed object with
+ an explicit data-type applied to all values.
+ oneOf:
+ - type: array
+ items:
+ $ref: '#/components/schemas/Literal'
+ - type: object
+ additionalProperties: false
+ required:
+ - type
+ - values
+ - data-type
+ properties:
+ type:
+ type: string
+ const: "literals"
+ values:
+ type: array
+ items:
+ $ref: '#/components/schemas/PrimitiveTypeValue'
+ data-type:
+ $ref: '#/components/schemas/PrimitiveType'
+
+ FieldReference:
+ description: >
+ A reference to a field. Either a bound reference (by field ID) or
+ an unbound reference (by name). The context in which an expression
+ is used determines which form is valid.
+ oneOf:
+ - $ref: '#/components/schemas/FieldReferenceById'
+ - $ref: '#/components/schemas/FieldReferenceByName'
+
+ FieldReferenceById:
+ description: A bound reference to a field by field ID.
+ type: object
+ additionalProperties: false
+ required:
+ - type
+ - id
+ properties:
+ type:
+ type: string
+ const: "reference"
+ id:
+ type: integer
+
+ FieldReferenceByName:
+ description: An unbound reference to a field by name.
+ type: object
+ additionalProperties: false
+ required:
+ - type
+ - name
+ properties:
+ type:
+ type: string
+ const: "reference"
+ name:
+ type: string
+
+ Apply:
+ description: >
+ A function application on zero or more value expressions or predicates.
+ type: object
+ additionalProperties: false
+ required:
+ - type
+ - function
+ - arguments
+ properties:
+ type:
+ type: string
+ const: "apply"
+ function:
+ $ref: '#/components/schemas/FunctionReference'
+ arguments:
+ type: array
+ items:
+ anyOf:
+ - $ref: '#/components/schemas/ValueExpression'
+ - $ref: '#/components/schemas/Expression'
+
+ FunctionReference:
+ description: >
+ A function reference is either a name, a multi-part name, an
+ identifier object, or a catalog-qualified identifier object.
+ oneOf:
+ - type: string
+ minLength: 1
+ - type: array
+ minItems: 1
+ items:
+ type: string
+ minLength: 1
+ - type: object
+ additionalProperties: false
+ required:
+ - identifier
+ properties:
+ identifier:
+ type: array
+ minItems: 1
+ items:
+ type: string
+ minLength: 1
+ - type: object
+ additionalProperties: false
+ required:
+ - catalog
+ - identifier
+ properties:
+ catalog:
+ type: string
+ identifier:
Review Comment:
Should this be `CatalogObjectIdentifier`?
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2606,7 +2630,205 @@ components:
child:
$ref: '#/components/schemas/Expression'
+ UnaryPredicate:
+ description: >
+ A predicate that tests a single value expression.
+ type: object
+ additionalProperties: false
+ required:
+ - type
+ - child
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["is-null", "not-null", "is-nan", "not-nan"]
+ child:
+ $ref: '#/components/schemas/ValueExpression'
+
+ ComparisonPredicate:
+ description: >
+ A predicate that compares two value expressions.
+ type: object
+ additionalProperties: false
+ required:
+ - type
+ - left
+ - right
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["lt", "lt-eq", "gt", "gt-eq", "eq", "not-eq", "starts-with", "not-starts-with"]
+ left:
+ $ref: '#/components/schemas/ValueExpression'
+ right:
+ $ref: '#/components/schemas/ValueExpression'
+
+ SetPredicate:
+ description: >
+ A predicate that operates on a list of literals.
+ type: object
+ additionalProperties: false
+ required:
+ - type
+ - child
+ - values
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["in", "not-in"]
+ child:
+ $ref: '#/components/schemas/ValueExpression'
+ values:
+ $ref: '#/components/schemas/Literals'
+
+ Literal:
+ description: >
+ A literal is either a bare value, an untyped literal object, or a
+ typed literal object.
+ oneOf:
+ - $ref: '#/components/schemas/PrimitiveTypeValue'
+ - type: object
+ additionalProperties: false
+ required:
+ - type
+ - value
+ properties:
+ type:
+ type: string
+ const: "literal"
+ value:
+ $ref: '#/components/schemas/PrimitiveTypeValue'
+ data-type:
+ $ref: '#/components/schemas/PrimitiveType'
+
+ Literals:
+ description: >
+ Literals is either a bare array of literals or a typed object with
+ an explicit data-type applied to all values.
+ oneOf:
+ - type: array
+ items:
+ $ref: '#/components/schemas/Literal'
+ - type: object
+ additionalProperties: false
+ required:
+ - type
+ - values
+ - data-type
+ properties:
+ type:
+ type: string
+ const: "literals"
+ values:
+ type: array
+ items:
+ $ref: '#/components/schemas/PrimitiveTypeValue'
+ data-type:
+ $ref: '#/components/schemas/PrimitiveType'
+
+ FieldReference:
+ description: >
+ A reference to a field. Either a bound reference (by field ID) or
+ an unbound reference (by name). The context in which an expression
+ is used determines which form is valid.
+ oneOf:
+ - $ref: '#/components/schemas/FieldReferenceById'
+ - $ref: '#/components/schemas/FieldReferenceByName'
+
+ FieldReferenceById:
+ description: A bound reference to a field by field ID.
+ type: object
+ additionalProperties: false
+ required:
+ - type
+ - id
+ properties:
+ type:
+ type: string
+ const: "reference"
+ id:
+ type: integer
+
+ FieldReferenceByName:
+ description: An unbound reference to a field by name.
+ type: object
+ additionalProperties: false
+ required:
+ - type
+ - name
+ properties:
+ type:
+ type: string
+ const: "reference"
+ name:
+ type: string
+
+ Apply:
+ description: >
+ A function application on zero or more value expressions or predicates.
+ type: object
+ additionalProperties: false
+ required:
+ - type
+ - function
+ - arguments
+ properties:
+ type:
+ type: string
+ const: "apply"
+ function:
+ $ref: '#/components/schemas/FunctionReference'
+ arguments:
+ type: array
+ items:
+ anyOf:
+ - $ref: '#/components/schemas/ValueExpression'
+ - $ref: '#/components/schemas/Expression'
+
+ FunctionReference:
+ description: >
+ A function reference is either a name, a multi-part name, an
+ identifier object, or a catalog-qualified identifier object.
+ oneOf:
+ - type: string
+ minLength: 1
+ - type: array
+ minItems: 1
+ items:
+ type: string
+ minLength: 1
+ - type: object
+ additionalProperties: false
+ required:
+ - identifier
+ properties:
+ identifier:
Review Comment:
`CatalogObjectIdentifier`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: Update REST spec expressions to match new expressions spec [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on PR #17138:
URL: https://github.com/apache/iceberg/pull/17138#issuecomment-5063288093
> I'm a little concerned about how we're evolving the spec here and what impact it may have on generated code. Do we care about compatibility at that level?
No, the generated Python code is for reference only. I was very much against adding it because I didn't want confusion over the guarantees for it. I agreed when we said it was not going to carry any guarantees.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: Update REST spec expressions to match new expressions spec [iceberg]
Posted by "amogh-jahagirdar (via GitHub)" <gi...@apache.org>.
amogh-jahagirdar commented on code in PR #17138:
URL: https://github.com/apache/iceberg/pull/17138#discussion_r3650469570
##########
open-api/rest-catalog-open-api.py:
##########
@@ -152,6 +152,11 @@ class ExpressionType(RootModel[str]):
class TrueExpression(BaseModel):
+ """
+ Deprecated. Use the bare boolean literal `true` as a predicate instead.
Review Comment:
Ok I think I take back what I said, we're trying to distinguish against the other literal `true`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: Update REST spec expressions to match new expressions spec [iceberg]
Posted by "singhpk234 (via GitHub)" <gi...@apache.org>.
singhpk234 commented on code in PR #17138:
URL: https://github.com/apache/iceberg/pull/17138#discussion_r3646786388
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2604,63 +2619,216 @@ components:
$ref: '#/components/schemas/ExpressionType'
const: "not"
child:
- $ref: '#/components/schemas/Expression'
+ $ref: '#/components/schemas/Predicate'
- UnaryExpression:
+ UnaryPredicate:
+ description: >
+ A predicate that tests a single value expression. Accepts either
+ 'child' (preferred) or 'term' (deprecated) to identify the operand.
type: object
+ additionalProperties: false
required:
- type
- - term
properties:
type:
$ref: '#/components/schemas/ExpressionType'
enum: ["is-null", "not-null", "is-nan", "not-nan"]
+ child:
+ $ref: '#/components/schemas/ValueExpression'
term:
+ deprecated: true
+ description: "Deprecated. Use 'child' instead."
$ref: '#/components/schemas/Term'
- LiteralExpression:
+ ComparisonPredicate:
+ description: >
+ A predicate that compares two value expressions. Accepts either
+ 'left'/'right' (preferred) or 'term'/'value' (deprecated).
type: object
+ additionalProperties: false
required:
Review Comment:
+1 to what ryan said, Please check this discussion : https://github.com/apache/iceberg/pull/17138#issuecomment-5063288093
the spec generated code can produce this half backed structure but spec text forbids it, and the concencus on this is to not rely on generated api from spec but use this to create implementation like we do in java api
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: Update REST spec expressions to match new expressions spec [iceberg]
Posted by "nevzheng (via GitHub)" <gi...@apache.org>.
nevzheng commented on code in PR #17138:
URL: https://github.com/apache/iceberg/pull/17138#discussion_r3562702560
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2787,10 +2787,9 @@ components:
FunctionReference:
description: >
- A reference to a function per Iceberg Expressions spec, Appendix B.
- Four JSON forms: a bare name (single-part identifier), a list of
- names (multi-part identifier), an object with an identifier, or an
- object with both a catalog and an identifier.
+ A reference to a function. Four JSON forms: a bare name (single-part
+ identifier), a list of names (multi-part identifier), an object with
Review Comment:
A function reference is either a name, a multi-part name, an identifier object, or a catalog-qualified identifier object.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: Update REST spec expressions to match new expressions spec [iceberg]
Posted by "nevzheng (via GitHub)" <gi...@apache.org>.
nevzheng commented on code in PR #17138:
URL: https://github.com/apache/iceberg/pull/17138#discussion_r3562699968
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2683,10 +2680,9 @@ components:
Literal:
description: >
- A literal value expression per Iceberg Expressions spec, Appendix B.
- Three JSON forms are accepted: a bare scalar value, a typed literal
- object without a data-type, or a typed literal object with an
- explicit data-type.
+ A literal value expression. Three JSON forms are accepted: a bare
+ scalar value, a typed literal object without a data-type, or a
Review Comment:
A literal is either a bare value, an untyped literal object, or a typed literal object.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: Update REST spec expressions to match new expressions spec [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #17138:
URL: https://github.com/apache/iceberg/pull/17138#discussion_r3591062548
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2606,7 +2630,205 @@ components:
child:
$ref: '#/components/schemas/Expression'
+ UnaryPredicate:
+ description: >
+ A predicate that tests a single value expression.
+ type: object
+ additionalProperties: false
+ required:
+ - type
+ - child
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["is-null", "not-null", "is-nan", "not-nan"]
+ child:
+ $ref: '#/components/schemas/ValueExpression'
+
+ ComparisonPredicate:
+ description: >
+ A predicate that compares two value expressions.
+ type: object
+ additionalProperties: false
+ required:
+ - type
+ - left
+ - right
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["lt", "lt-eq", "gt", "gt-eq", "eq", "not-eq", "starts-with", "not-starts-with"]
+ left:
+ $ref: '#/components/schemas/ValueExpression'
+ right:
+ $ref: '#/components/schemas/ValueExpression'
+
+ SetPredicate:
+ description: >
+ A predicate that operates on a list of literals.
+ type: object
+ additionalProperties: false
+ required:
+ - type
+ - child
+ - values
+ properties:
+ type:
+ $ref: '#/components/schemas/ExpressionType'
+ enum: ["in", "not-in"]
+ child:
+ $ref: '#/components/schemas/ValueExpression'
+ values:
+ $ref: '#/components/schemas/Literals'
+
+ Literal:
+ description: >
+ A literal is either a bare value, an untyped literal object, or a
+ typed literal object.
+ oneOf:
+ - $ref: '#/components/schemas/PrimitiveTypeValue'
+ - type: object
+ additionalProperties: false
+ required:
+ - type
+ - value
+ properties:
+ type:
+ type: string
+ const: "literal"
+ value:
+ $ref: '#/components/schemas/PrimitiveTypeValue'
+ data-type:
+ $ref: '#/components/schemas/PrimitiveType'
+
+ Literals:
+ description: >
+ Literals is either a bare array of literals or a typed object with
+ an explicit data-type applied to all values.
+ oneOf:
+ - type: array
+ items:
+ $ref: '#/components/schemas/Literal'
+ - type: object
+ additionalProperties: false
+ required:
+ - type
+ - values
+ - data-type
+ properties:
+ type:
+ type: string
+ const: "literals"
+ values:
+ type: array
+ items:
+ $ref: '#/components/schemas/PrimitiveTypeValue'
+ data-type:
+ $ref: '#/components/schemas/PrimitiveType'
+
+ FieldReference:
+ description: >
+ A reference to a field. Either a bound reference (by field ID) or
+ an unbound reference (by name). The context in which an expression
+ is used determines which form is valid.
+ oneOf:
+ - $ref: '#/components/schemas/FieldReferenceById'
+ - $ref: '#/components/schemas/FieldReferenceByName'
+
+ FieldReferenceById:
Review Comment:
Why not `IdReference` and `NamedReference`? These names are just a bit longer than needed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: Update REST spec expressions to match new expressions spec [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #17138:
URL: https://github.com/apache/iceberg/pull/17138#discussion_r3591052848
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2606,7 +2630,205 @@ components:
child:
$ref: '#/components/schemas/Expression'
+ UnaryPredicate:
Review Comment:
Similar to my comment below, can we avoid having `UnaryPredicate` and `UnaryExpression` (deprecated) by allowing a `term` field here?
I know that this `UnaryPredicate` restricts the `child` field to `ValueExpression` and `UnaryExpression` used `Term`, but `Term` should actually be a subset of what you can express with `ValueExpression`.
`Term` was a `Reference` (named) or a transform. Both of those can be returned as `NamedReference` and `Apply` at read time.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: Update REST spec expressions to match new expressions spec [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #17138:
URL: https://github.com/apache/iceberg/pull/17138#discussion_r3591150500
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2530,13 +2530,17 @@ components:
Expression:
oneOf:
+ - type: boolean
- $ref: '#/components/schemas/TrueExpression'
- $ref: '#/components/schemas/FalseExpression'
- $ref: '#/components/schemas/AndOrExpression'
- $ref: '#/components/schemas/NotExpression'
- - $ref: '#/components/schemas/SetExpression'
- - $ref: '#/components/schemas/LiteralExpression'
+ - $ref: '#/components/schemas/UnaryPredicate'
+ - $ref: '#/components/schemas/ComparisonPredicate'
+ - $ref: '#/components/schemas/SetPredicate'
- $ref: '#/components/schemas/UnaryExpression'
+ - $ref: '#/components/schemas/LiteralExpression'
+ - $ref: '#/components/schemas/SetExpression'
Review Comment:
Can we rename this to `Predicate`? I think that would be more clear. I think we are probably also renaming a couple of other structures (assuming that we combine `UnaryExpression` with `UnaryPredicate` and similar) so we could clean up quite a bit this way.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org