You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by GitBox <gi...@apache.org> on 2021/02/19 16:01:39 UTC
[GitHub] [calcite] rubenada opened a new pull request #2353: [CALCITE-2000] UNNEST a collection that has a field with nested data generates an Exception
rubenada opened a new pull request #2353:
URL: https://github.com/apache/calcite/pull/2353
Jira: [CALCITE-2000](https://issues.apache.org/jira/browse/CALCITE-2000)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] vlsi commented on a change in pull request #2353: [CALCITE-2000] UNNEST a collection that has a field with nested data generates an Exception
Posted by GitBox <gi...@apache.org>.
vlsi commented on a change in pull request #2353:
URL: https://github.com/apache/calcite/pull/2353#discussion_r583607305
##########
File path: core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
##########
@@ -2792,27 +2792,28 @@ public static Collection multisetUnionAll(Collection collection1,
* Function that, given a certain List containing single-item structs (i.e. arrays / lists with
* a single item), builds an Enumerable that returns those single items inside the structs.
*/
- public static Function1<Object, Enumerable<Comparable>> flatList() {
+ public static Function1<Object, Enumerable<Object>> flatList() {
Review comment:
Are `null` values acceptable for the resulting function?
In other words, is `inputObject == null` valid or not?
If it is valid, then the signature should better be
What do you think of the following signature?
```suggestion
public static <T> Function1<T, Enumerable<T>> flatList() {
```
If null is not valid, then cast from `inputObject` to `List` should indeed be like `(List<Object>) ...` which effectively means `the list of non-nullable 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on a change in pull request #2353: [CALCITE-2000] UNNEST a collection that has a field with nested data generates an Exception
Posted by GitBox <gi...@apache.org>.
rubenada commented on a change in pull request #2353:
URL: https://github.com/apache/calcite/pull/2353#discussion_r583635078
##########
File path: core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
##########
@@ -2792,27 +2792,28 @@ public static Collection multisetUnionAll(Collection collection1,
* Function that, given a certain List containing single-item structs (i.e. arrays / lists with
* a single item), builds an Enumerable that returns those single items inside the structs.
*/
- public static Function1<Object, Enumerable<Comparable>> flatList() {
+ public static Function1<Object, Enumerable<Object>> flatList() {
Review comment:
`SqlFunction#flatList` was introduced in 1.24 via CALCITE-4063, and it is a counter-part of `flatProduct` for a very particular case. So, following `flatProduct`'s logic, inputObject cannot be null, so I guess `flatList` should look like:
```
public static Function1<Object, Enumerable<Object>> flatList() {
return inputObject -> {
//noinspection unchecked
final List<Object> list = (List<Object>) inputObject;
final Enumerator<Object> enumerator = Linq4j.enumerator(list);
...
```
do you agree?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on pull request #2353: [CALCITE-2000] UNNEST a collection that has a field with nested data generates an Exception
Posted by GitBox <gi...@apache.org>.
rubenada commented on pull request #2353:
URL: https://github.com/apache/calcite/pull/2353#issuecomment-787772605
Thanks for your help @vlsi !
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on a change in pull request #2353: [CALCITE-2000] UNNEST a collection that has a field with nested data generates an Exception
Posted by GitBox <gi...@apache.org>.
rubenada commented on a change in pull request #2353:
URL: https://github.com/apache/calcite/pull/2353#discussion_r583669896
##########
File path: core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
##########
@@ -2792,27 +2792,28 @@ public static Collection multisetUnionAll(Collection collection1,
* Function that, given a certain List containing single-item structs (i.e. arrays / lists with
* a single item), builds an Enumerable that returns those single items inside the structs.
*/
- public static Function1<Object, Enumerable<Comparable>> flatList() {
+ public static Function1<Object, Enumerable<Object>> flatList() {
Review comment:
Done. Definitely better, thanks for the tip!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] vlsi commented on a change in pull request #2353: [CALCITE-2000] UNNEST a collection that has a field with nested data generates an Exception
Posted by GitBox <gi...@apache.org>.
vlsi commented on a change in pull request #2353:
URL: https://github.com/apache/calcite/pull/2353#discussion_r583679658
##########
File path: core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
##########
@@ -2792,40 +2792,8 @@ public static Collection multisetUnionAll(Collection collection1,
* Function that, given a certain List containing single-item structs (i.e. arrays / lists with
* a single item), builds an Enumerable that returns those single items inside the structs.
*/
- public static Function1<Object, Enumerable<Comparable>> flatList() {
- return inputObject -> {
- final List list = (List) inputObject;
- final Enumerator<List<Object>> enumerator = Linq4j.enumerator(list);
- return new AbstractEnumerable<Comparable>() {
- @Override public Enumerator<Comparable> enumerator() {
- return new Enumerator<Comparable>() {
-
- @Override public boolean moveNext() {
- return enumerator.moveNext();
- }
-
- @Override public Comparable current() {
- final Object element = enumerator.current();
- final Comparable comparable;
- if (element.getClass().isArray()) {
- comparable = (Comparable) ((Object[]) element)[0];
- } else {
- comparable = (Comparable) ((List) element).get(0);
- }
- return comparable;
- }
-
- @Override public void reset() {
- enumerator.reset();
- }
-
- @Override public void close() {
- enumerator.close();
- }
- };
- }
- };
- };
+ public static Function1<List<Object>, Enumerable<Object>> flatList() {
+ return inputList -> Linq4j.asEnumerable(inputList).select(v -> structAccess(v, 0, null));
Review comment:
Great. Now it is much easier to understand what this call does :)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada merged pull request #2353: [CALCITE-2000] UNNEST a collection that has a field with nested data generates an Exception
Posted by GitBox <gi...@apache.org>.
rubenada merged pull request #2353:
URL: https://github.com/apache/calcite/pull/2353
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] vlsi commented on a change in pull request #2353: [CALCITE-2000] UNNEST a collection that has a field with nested data generates an Exception
Posted by GitBox <gi...@apache.org>.
vlsi commented on a change in pull request #2353:
URL: https://github.com/apache/calcite/pull/2353#discussion_r583679658
##########
File path: core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
##########
@@ -2792,40 +2792,8 @@ public static Collection multisetUnionAll(Collection collection1,
* Function that, given a certain List containing single-item structs (i.e. arrays / lists with
* a single item), builds an Enumerable that returns those single items inside the structs.
*/
- public static Function1<Object, Enumerable<Comparable>> flatList() {
- return inputObject -> {
- final List list = (List) inputObject;
- final Enumerator<List<Object>> enumerator = Linq4j.enumerator(list);
- return new AbstractEnumerable<Comparable>() {
- @Override public Enumerator<Comparable> enumerator() {
- return new Enumerator<Comparable>() {
-
- @Override public boolean moveNext() {
- return enumerator.moveNext();
- }
-
- @Override public Comparable current() {
- final Object element = enumerator.current();
- final Comparable comparable;
- if (element.getClass().isArray()) {
- comparable = (Comparable) ((Object[]) element)[0];
- } else {
- comparable = (Comparable) ((List) element).get(0);
- }
- return comparable;
- }
-
- @Override public void reset() {
- enumerator.reset();
- }
-
- @Override public void close() {
- enumerator.close();
- }
- };
- }
- };
- };
+ public static Function1<List<Object>, Enumerable<Object>> flatList() {
+ return inputList -> Linq4j.asEnumerable(inputList).select(v -> structAccess(v, 0, null));
Review comment:
Great. Now it is much easier to understand what this code does :)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on a change in pull request #2353: [CALCITE-2000] UNNEST a collection that has a field with nested data generates an Exception
Posted by GitBox <gi...@apache.org>.
rubenada commented on a change in pull request #2353:
URL: https://github.com/apache/calcite/pull/2353#discussion_r583635078
##########
File path: core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
##########
@@ -2792,27 +2792,28 @@ public static Collection multisetUnionAll(Collection collection1,
* Function that, given a certain List containing single-item structs (i.e. arrays / lists with
* a single item), builds an Enumerable that returns those single items inside the structs.
*/
- public static Function1<Object, Enumerable<Comparable>> flatList() {
+ public static Function1<Object, Enumerable<Object>> flatList() {
Review comment:
{{SqlFunction#flatList}} was introduced in 1.24 via CALCITE-4063, and it is a counter-part of {{flatProduct}} for a very particular case. So, following {{flatProduct}}'s logic, inputObject cannot be null, so I guess {{flatList}} should look like:
```
public static Function1<Object, Enumerable<Object>> flatList() {
return inputObject -> {
//noinspection unchecked
final List<Object> list = (List<Object>) inputObject;
final Enumerator<Object> enumerator = Linq4j.enumerator(list);
...
```
do you agree?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] vlsi commented on a change in pull request #2353: [CALCITE-2000] UNNEST a collection that has a field with nested data generates an Exception
Posted by GitBox <gi...@apache.org>.
vlsi commented on a change in pull request #2353:
URL: https://github.com/apache/calcite/pull/2353#discussion_r583652750
##########
File path: core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
##########
@@ -2792,27 +2792,28 @@ public static Collection multisetUnionAll(Collection collection1,
* Function that, given a certain List containing single-item structs (i.e. arrays / lists with
* a single item), builds an Enumerable that returns those single items inside the structs.
*/
- public static Function1<Object, Enumerable<Comparable>> flatList() {
+ public static Function1<Object, Enumerable<Object>> flatList() {
Review comment:
Yes, that would be better indeed.
PS. `new Abstract ...` could probably be replaced with `Linq4j.asEnumerable(inputList).select(v -> ...)`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] vlsi commented on a change in pull request #2353: [CALCITE-2000] UNNEST a collection that has a field with nested data generates an Exception
Posted by GitBox <gi...@apache.org>.
vlsi commented on a change in pull request #2353:
URL: https://github.com/apache/calcite/pull/2353#discussion_r583652750
##########
File path: core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
##########
@@ -2792,27 +2792,28 @@ public static Collection multisetUnionAll(Collection collection1,
* Function that, given a certain List containing single-item structs (i.e. arrays / lists with
* a single item), builds an Enumerable that returns those single items inside the structs.
*/
- public static Function1<Object, Enumerable<Comparable>> flatList() {
+ public static Function1<Object, Enumerable<Object>> flatList() {
Review comment:
Yes, that would be better indeed.
PS. `new Abstract ...` could probably be replaced with `Linq4j.asEnumerable(inputList).select(v -> structAccess(v, 0, null))`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] vlsi commented on pull request #2353: [CALCITE-2000] UNNEST a collection that has a field with nested data generates an Exception
Posted by GitBox <gi...@apache.org>.
vlsi commented on pull request #2353:
URL: https://github.com/apache/calcite/pull/2353#issuecomment-787764254
I think it is good to go
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on a change in pull request #2353: [CALCITE-2000] UNNEST a collection that has a field with nested data generates an Exception
Posted by GitBox <gi...@apache.org>.
rubenada commented on a change in pull request #2353:
URL: https://github.com/apache/calcite/pull/2353#discussion_r583669761
##########
File path: core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
##########
@@ -2792,27 +2792,26 @@ public static Collection multisetUnionAll(Collection collection1,
* Function that, given a certain List containing single-item structs (i.e. arrays / lists with
* a single item), builds an Enumerable that returns those single items inside the structs.
*/
- public static Function1<Object, Enumerable<Comparable>> flatList() {
- return inputObject -> {
- final List list = (List) inputObject;
- final Enumerator<List<Object>> enumerator = Linq4j.enumerator(list);
- return new AbstractEnumerable<Comparable>() {
- @Override public Enumerator<Comparable> enumerator() {
- return new Enumerator<Comparable>() {
+ public static Function1<List<Object>, Enumerable<Object>> flatList() {
+ return inputList -> {
+ final Enumerator<Object> enumerator = Linq4j.enumerator(inputList);
+ return new AbstractEnumerable<Object>() {
+ @Override public Enumerator<Object> enumerator() {
+ return new Enumerator<Object>() {
@Override public boolean moveNext() {
return enumerator.moveNext();
}
- @Override public Comparable current() {
+ @Override public Object current() {
final Object element = enumerator.current();
- final Comparable comparable;
+ final Object current;
if (element.getClass().isArray()) {
- comparable = (Comparable) ((Object[]) element)[0];
+ current = ((Object[]) element)[0];
} else {
- comparable = (Comparable) ((List) element).get(0);
+ current = ((List) element).get(0);
}
Review comment:
Done.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on pull request #2353: [CALCITE-2000] UNNEST a collection that has a field with nested data generates an Exception
Posted by GitBox <gi...@apache.org>.
rubenada commented on pull request #2353:
URL: https://github.com/apache/calcite/pull/2353#issuecomment-786607053
@vlsi any chance you can get a look at this one, please? I'd like to get this fixed for 1.27.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] vlsi commented on a change in pull request #2353: [CALCITE-2000] UNNEST a collection that has a field with nested data generates an Exception
Posted by GitBox <gi...@apache.org>.
vlsi commented on a change in pull request #2353:
URL: https://github.com/apache/calcite/pull/2353#discussion_r583607305
##########
File path: core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
##########
@@ -2792,27 +2792,28 @@ public static Collection multisetUnionAll(Collection collection1,
* Function that, given a certain List containing single-item structs (i.e. arrays / lists with
* a single item), builds an Enumerable that returns those single items inside the structs.
*/
- public static Function1<Object, Enumerable<Comparable>> flatList() {
+ public static Function1<Object, Enumerable<Object>> flatList() {
Review comment:
Are `null` values acceptable for the resulting function?
In other words, is `inputObject == null` valid or not?
If it is valid, then the signature should better be
```java
What do you think of the following signature?
```suggestion
public static <T> Function1<T, Enumerable<T>> flatList() {
```
If null is not valid, then cast from `inputObject` to `List` should indeed be like `(List<Object>) ...` which effectively means `the list of non-nullable 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] vlsi commented on a change in pull request #2353: [CALCITE-2000] UNNEST a collection that has a field with nested data generates an Exception
Posted by GitBox <gi...@apache.org>.
vlsi commented on a change in pull request #2353:
URL: https://github.com/apache/calcite/pull/2353#discussion_r583654866
##########
File path: core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
##########
@@ -2792,27 +2792,26 @@ public static Collection multisetUnionAll(Collection collection1,
* Function that, given a certain List containing single-item structs (i.e. arrays / lists with
* a single item), builds an Enumerable that returns those single items inside the structs.
*/
- public static Function1<Object, Enumerable<Comparable>> flatList() {
- return inputObject -> {
- final List list = (List) inputObject;
- final Enumerator<List<Object>> enumerator = Linq4j.enumerator(list);
- return new AbstractEnumerable<Comparable>() {
- @Override public Enumerator<Comparable> enumerator() {
- return new Enumerator<Comparable>() {
+ public static Function1<List<Object>, Enumerable<Object>> flatList() {
+ return inputList -> {
+ final Enumerator<Object> enumerator = Linq4j.enumerator(inputList);
+ return new AbstractEnumerable<Object>() {
+ @Override public Enumerator<Object> enumerator() {
+ return new Enumerator<Object>() {
@Override public boolean moveNext() {
return enumerator.moveNext();
}
- @Override public Comparable current() {
+ @Override public Object current() {
final Object element = enumerator.current();
- final Comparable comparable;
+ final Object current;
if (element.getClass().isArray()) {
- comparable = (Comparable) ((Object[]) element)[0];
+ current = ((Object[]) element)[0];
} else {
- comparable = (Comparable) ((List) element).get(0);
+ current = ((List) element).get(0);
}
Review comment:
What do you think of reusing `org.apache.calcite.runtime.SqlFunctions#structAccess` 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on a change in pull request #2353: [CALCITE-2000] UNNEST a collection that has a field with nested data generates an Exception
Posted by GitBox <gi...@apache.org>.
rubenada commented on a change in pull request #2353:
URL: https://github.com/apache/calcite/pull/2353#discussion_r583647298
##########
File path: core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
##########
@@ -2792,27 +2792,28 @@ public static Collection multisetUnionAll(Collection collection1,
* Function that, given a certain List containing single-item structs (i.e. arrays / lists with
* a single item), builds an Enumerable that returns those single items inside the structs.
*/
- public static Function1<Object, Enumerable<Comparable>> flatList() {
+ public static Function1<Object, Enumerable<Object>> flatList() {
Review comment:
Actually, I think we can do even better / simpler:
```
public static Function1<List<Object>, Enumerable<Object>> flatList() {
return inputList -> {
final Enumerator<Object> enumerator = Linq4j.enumerator(inputList);
...
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on a change in pull request #2353: [CALCITE-2000] UNNEST a collection that has a field with nested data generates an Exception
Posted by GitBox <gi...@apache.org>.
rubenada commented on a change in pull request #2353:
URL: https://github.com/apache/calcite/pull/2353#discussion_r583647298
##########
File path: core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
##########
@@ -2792,27 +2792,28 @@ public static Collection multisetUnionAll(Collection collection1,
* Function that, given a certain List containing single-item structs (i.e. arrays / lists with
* a single item), builds an Enumerable that returns those single items inside the structs.
*/
- public static Function1<Object, Enumerable<Comparable>> flatList() {
+ public static Function1<Object, Enumerable<Object>> flatList() {
Review comment:
Actually, I think we can do even better / simpler:
```
public static Function1<List<Object>, Enumerable<Object>> flatList() {
return inputObject -> {
final Enumerator<Object> enumerator = Linq4j.enumerator(inputObject);
...
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on pull request #2353: [CALCITE-2000] UNNEST a collection that has a field with nested data generates an Exception
Posted by GitBox <gi...@apache.org>.
rubenada commented on pull request #2353:
URL: https://github.com/apache/calcite/pull/2353#issuecomment-787759707
@vlsi do you have any other comment about this PR? Do you think it is in a good shape for merge?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org