Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FLINK-35423][table] ARRAY_EXCEPT should follow set semantics #24828

Merged
merged 6 commits into from
Jun 7, 2024

Conversation

snuyanzin
Copy link
Contributor

What is the purpose of the change

The idea of the PR is making of ARRAY_EXCEPT following set semantics

Brief change log

flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/functions/CollectionFunctionsITCase.java
flink-table/flink-table-runtime/src/main/java/org/apache/flink/table/runtime/functions/scalar/ArrayExceptFunction.java

Verifying this change

flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/functions/CollectionFunctionsITCase.java

Does this pull request potentially affect one of the following parts:

  • Dependencies (does it add or upgrade a dependency): ( no)
  • The public API, i.e., is any changed class annotated with @Public(Evolving): ( no)
  • The serializers: ( no )
  • The runtime per-record code paths (performance sensitive): (no )
  • Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: (no )
  • The S3 file system connector: (no )

Documentation

  • Does this pull request introduce a new feature? (yes / no)
  • If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)

@flinkbot
Copy link
Collaborator

flinkbot commented May 22, 2024

CI report:

Bot commands The @flinkbot bot supports the following commands:
  • @flinkbot run azure re-run the last Azure build

@reswqa
Copy link
Member

reswqa commented Jun 4, 2024

Hi @snuyanzin, what is the state of this PR right now? I notice this as it marked as a blocker of 1.20.

@snuyanzin
Copy link
Contributor Author

hi @reswqa thanks for reaching out
seems I missed this test failure, thanks for noticing this
I updated the test, it should pass

now just need to wait and then need for a reviewer

Copy link
Contributor

@xuyangzhong xuyangzhong left a comment

Choose a reason for hiding this comment

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

LGTM, just one minor comment.

@@ -687,7 +687,7 @@ collection:
description: Returns a map created from an arrays of keys and values. Note that the lengths of two arrays should be the same.
- sql: ARRAY_EXCEPT(array1, array2)
table: arrayOne.arrayExcept(arrayTwo)
description: Returns an ARRAY that contains the elements from array1 that are not in array2. If no elements remain after excluding the elements in array2 from array1, the function returns an empty ARRAY. If one or both arguments are NULL, the function returns NULL. The order of the elements from array1 is kept.
description: Returns an ARRAY that contains the elements from array1 that are not in array2. If no elements remain after excluding the elements in array2 from array1, the function returns an empty ARRAY. If one or both arguments are NULL, the function returns NULL. The order of the elements from array1 is kept however the duplicates are removed.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: what about following the format with 'ARRAY_UNION'?

Returns an ARRAY that contains the elements from array1 that are not in array2, without duplicates.
...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep, that looks more concise and easier to read

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, perhaps I haven't made myself clear, I want to keep the latter part the same.

Returns an ARRAY that contains the elements from array1 that are not in array2, without duplicates. If no elements remain after excluding the elements in array2 from array1, the function returns an empty ARRAY. If one or both arguments are NULL, the function returns NULL. The order of the elements from array1 is kept.

Copy link
Contributor

@xuyangzhong xuyangzhong left a comment

Choose a reason for hiding this comment

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

Thanks for this update.

@snuyanzin
Copy link
Contributor Author

rebased to solve the conflicts

thanks for the review, once it's green I will merge it

@@ -817,6 +817,12 @@ collection:
- sql: MAP_UNION(map1, map2)
table: map1.mapUnion(map2)
description: 返回一个通过合并两个图 'map1' 和 'map2' 创建的图。这两个图应该具有共同的图类型。如果有重叠的键,'map2' 的值将覆盖 'map1' 的值。如果任一图为空,则返回 null。
- sql: ARRAY_EXCEPT(array1, array2)
table: arrayOne.arrayExcept(arrayTwo)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: may put it to arrays function better

@liuyongvs
Copy link
Contributor

liuyongvs commented Jun 7, 2024

LGTM, only left minor comments. and arrays function put them together and ordered by name may be better

@snuyanzin
Copy link
Contributor Author

Thanks for taking a look

@snuyanzin snuyanzin merged commit 0305111 into apache:master Jun 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants