You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "liuyongvs (via GitHub)" <gi...@apache.org> on 2023/03/10 03:08:10 UTC
[GitHub] [flink] liuyongvs opened a new pull request, #22143: [FLINK-31377][table] Fix array_contains ArrayData.ElementGetter shoul…
liuyongvs opened a new pull request, #22143:
URL: https://github.com/apache/flink/pull/22143
## What is the purpose of the change
*Fix array_contains ArrayData.ElementGetter should use element type instead of needle type.*
- needle and element type are identical. because " a NOT NULL type can be stored in NULL type but not vice versa." and after dig the code TypeInferenceOperandChecker.insertImplicitCasts, you can see here supportsAvoidingCast.
`
// code placeholder
Stream<TestSetSpec> getTestSetSpecs() {
return Stream.of(
TestSetSpec.forFunction(BuiltInFunctionDefinitions.ARRAY_CONTAINS)
.onFieldsWithData(
new Map[] {
null,
CollectionUtil.map(entry(1, "a"), entry(2, "b")),
CollectionUtil.map(entry(3, "c"), entry(4, "d")),
},
null)
.andDataTypes(
DataTypes.ARRAY(DataTypes.MAP(DataTypes.INT(), DataTypes.STRING())),
DataTypes.STRING())
.testResult(
$("f0").arrayContains(
CollectionUtil.map(entry(3, "c"), entry(4, "d"))),
"ARRAY_CONTAINS(f0, MAP[3, 'c', 4, 'd'])",
true,
DataTypes.BOOLEAN()));
}
`
## Verifying this change
- add unit test
## 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): yes
- 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/Mesos, ZooKeeper: no
- The S3 file system connector: no
## Documentation
- Does this pull request introduce a new feature? yes
- If yes, how is the feature documented? docs / JavaDocs
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] liuyongvs commented on pull request #22143: [FLINK-31377][table] Fix array_contains ArrayData.ElementGetter shoul…
Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.
liuyongvs commented on PR #22143:
URL: https://github.com/apache/flink/pull/22143#issuecomment-1583953198
hi @luoyuxia will you have time to review it?
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] snuyanzin commented on pull request #22143: [FLINK-31377][table] Fix array_contains ArrayData.ElementGetter shoul…
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on PR #22143:
URL: https://github.com/apache/flink/pull/22143#issuecomment-1584396536
Well i don't see a good reason to fix something that could be checked only via artifitial test-case and at the same time keep not working on SQL level...
In my opinion it's better to have both working or both not-working
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] liuyongvs commented on pull request #22143: [FLINK-31377][table] Fix array_contains ArrayData.ElementGetter shoul…
Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.
liuyongvs commented on PR #22143:
URL: https://github.com/apache/flink/pull/22143#issuecomment-1587101608
hi @wuchong what do you think?
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] liuyongvs commented on pull request #22143: [FLINK-31377][table] Fix array_contains ArrayData.ElementGetter shoul…
Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.
liuyongvs commented on PR #22143:
URL: https://github.com/apache/flink/pull/22143#issuecomment-1586689922
@snuyanzin another example:
if user use it like this, the b column is a udx or a builtin function, which using BinayArrayData. it will also throw exception
```
SELECT array_contains(b, map[1, 'a']);
```
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] snuyanzin commented on pull request #22143: [FLINK-31377][table] Fix array_contains ArrayData.ElementGetter shoul…
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on PR #22143:
URL: https://github.com/apache/flink/pull/22143#issuecomment-1584413439
i try to have a look from user point of view:
now it does not work both via api and SQL
imagine after fixing this it will work via API however not via SQL and that introducies inconcistency in behavior
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] liuyongvs commented on pull request #22143: [FLINK-31377][table] Fix array_contains ArrayData.ElementGetter shoul…
Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.
liuyongvs commented on PR #22143:
URL: https://github.com/apache/flink/pull/22143#issuecomment-1584527028
@snuyanzin one more thing
1. It is bug array_contains
2. the issues https://issues.apache.org/jira/browse/FLINK-27438 is about array[map]. Totally different problem, why we also put them together to taklk
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] liuyongvs commented on pull request #22143: [FLINK-31377][table] Fix array_contains ArrayData.ElementGetter shoul…
Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.
liuyongvs commented on PR #22143:
URL: https://github.com/apache/flink/pull/22143#issuecomment-1584281558
@snuyanzin i debug it, the sql way you offers, it is GenericRowData,
while unit test in local debug it is BinaryRowData,that is why your example not throw exception
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] liuyongvs commented on pull request #22143: [FLINK-31377][table] Fix array_contains ArrayData.ElementGetter shoul…
Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.
liuyongvs commented on PR #22143:
URL: https://github.com/apache/flink/pull/22143#issuecomment-1584403618
@snuyanzin Ha ha, may we fix one thing in one pr in the software development principal. just my personal think.
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] snuyanzin commented on pull request #22143: [FLINK-31377][table] Fix array_contains ArrayData.ElementGetter shoul…
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on PR #22143:
URL: https://github.com/apache/flink/pull/22143#issuecomment-1584165059
That's why is table definition there are both
```
...
'fields.b.null-rate' = '0.5',
'fields.b.element.null-rate' = '0.5'
...
```
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] flinkbot commented on pull request #22143: [FLINK-31377][table] Fix array_contains ArrayData.ElementGetter shoul…
Posted by "flinkbot (via GitHub)" <gi...@apache.org>.
flinkbot commented on PR #22143:
URL: https://github.com/apache/flink/pull/22143#issuecomment-1463191208
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "2a2f0a9392c16f8d52dea8afcad66e5b13015ffd",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "2a2f0a9392c16f8d52dea8afcad66e5b13015ffd",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 2a2f0a9392c16f8d52dea8afcad66e5b13015ffd UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run azure` re-run the last Azure build
</details>
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] liuyongvs commented on pull request #22143: [FLINK-31377][table] Fix array_contains ArrayData.ElementGetter shoul…
Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.
liuyongvs commented on PR #22143:
URL: https://github.com/apache/flink/pull/22143#issuecomment-1584386417
@snuyanzin but it is a different problem. that is to say, there are 2 problem. this https://issues.apache.org/jira/browse/FLINK-27438 i reported before in the issue. so i think we should fix the first one . then fix https://issues.apache.org/jira/browse/FLINK-27438
what do you think?
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] liuyongvs commented on pull request #22143: [FLINK-31377][table] Fix array_contains ArrayData.ElementGetter shoul…
Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.
liuyongvs commented on PR #22143:
URL: https://github.com/apache/flink/pull/22143#issuecomment-1584440629
@snuyanzin
1) first, this https://issues.apache.org/jira/browse/FLINK-27438 is long time, and don't have any progress.
2) from my side. this is 2 different problem indeed. we need to fix bugs try out bese.
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] liuyongvs commented on pull request #22143: [FLINK-31377][table] Fix array_contains ArrayData.ElementGetter shoul…
Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.
liuyongvs commented on PR #22143:
URL: https://github.com/apache/flink/pull/22143#issuecomment-1584119205
and that is why i add this test, it should contains the null elements, when it throw exception
```
new Map[] {
null,
CollectionUtil.map(entry(1, "a"), entry(2, "b")),
CollectionUtil.map(entry(3, "c"), entry(4, "d")),
}
```
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] snuyanzin commented on pull request #22143: [FLINK-31377][table] Fix array_contains ArrayData.ElementGetter shoul…
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on PR #22143:
URL: https://github.com/apache/flink/pull/22143#issuecomment-1584147817
@liuyongvs thanks for your response
I tried to do it with datagen (since recently it supports `null-rate`), so I tried this
```sql
CREATE TABLE t (
a BIGINT,
b ARRAY<MAP<INT, STRING>>
) WITH (
'connector' = 'datagen',
'fields.a.null-rate' = '0.5',
'fields.b.null-rate' = '0.5',
'fields.b.element.null-rate' = '0.5'
);
```
and then this query (you've mentioned)
```sql
SELECT a, b, ARRAY_CONTAINS(b, MAP[1, 'a']) FROM t;
```
for about 5 min I was not able to get any exception...
About test in the PR:
I'm a bit skeptic about it because it explicitly sets entry of map to `NULL`. What functionality allows to that?
I do not remember any SQL function allowing to do that. Key and value yes however not the whole entry
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] liuyongvs commented on pull request #22143: [FLINK-31377][table] Fix array_contains ArrayData.ElementGetter shoul…
Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.
liuyongvs commented on PR #22143:
URL: https://github.com/apache/flink/pull/22143#issuecomment-1584162518
@snuyanzin b.null-rate just b is null, but it is element is null. which is different. it should array[null, map[1, 'a']] like this
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] liuyongvs commented on pull request #22143: [FLINK-31377][table] Fix array_contains ArrayData.ElementGetter shoul…
Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.
liuyongvs commented on PR #22143:
URL: https://github.com/apache/flink/pull/22143#issuecomment-1584115236
hi @snuyanzin why do not you debug it, the test i offers in the pr. why not offer pure sql , i don't find a good way to offer.
for example.
```
CREATE TABLE t1 (
a BIGINT,
b ARRAY<MAP<INT, STRING>>
) WITH (
'connector' = 'datagen'
);
select array_contains(b, map[1, 'a']) from t1;
when it throw exception, should be has element null. how do i offer? the easy way i think it is in by values connector to register data, which is also code instead of pure sql
```
```
val myTableDataId = TestValuesTableFactory.registerRowData(values)
val ddl =
s"""
|CREATE TABLE CustomTable (
| a bigint,
| b map<bigint, bigint>
|) WITH (
| 'connector' = 'values',
| 'data-id' = '$myTableDataId',
| 'register-internal-data' = 'true',
| 'bounded' = 'true'
|)
""".stripMargin
```
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink] liuyongvs commented on pull request #22143: [FLINK-31377][table] Fix array_contains ArrayData.ElementGetter shoul…
Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.
liuyongvs commented on PR #22143:
URL: https://github.com/apache/flink/pull/22143#issuecomment-1592862067
hi @snuyanzin do you have time to have a look again?
--
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@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org