[CALCITE-6620] VALUES created by RelBuilder do not have a homogeneous type - #4000
Conversation
| f0.forEachLibrary(libraries, consumer); | ||
| } | ||
|
|
||
| @Test void differentTypeValues() { |
There was a problem hiding this comment.
Does this test belong here? It isn't testing an operator, or use the fixture.
Could it be a Quidem test?
There was a problem hiding this comment.
I will move it.
I don't know if I can create the same environment in Quidem. A simple attempt to reproduce this bug with standard settings failed.
| RelDataType rowType, List<RexLiteral> values) { | ||
| assert values.size() == rowType.getFieldCount(); | ||
| RexExecutor executor = cluster.getPlanner().getExecutor(); | ||
| if (executor == null) { |
There was a problem hiding this comment.
Do we have a test for the executor == null？I'm not sure why this happened.
There was a problem hiding this comment.
Who knows who is calling the builder and with what setup?
This is just to be safe.
There was a problem hiding this comment.
+1, getExecutor is a nullable method.
There was a problem hiding this comment.
Maybe we can improve it by RexExecutor rexImpl = Util.first(cluster.getPlanner().getExecutor(), RexUtil.EXECUTOR);
|
Several quidem tests now have extra spaces in strings because they use a type system without raggedUnionTypesToVarying. The extra spaces are introduced when unifying the types of the VALUES. The new results seem the right ones. |
| RelDataType rowType, List<RexLiteral> values) { | ||
| assert values.size() == rowType.getFieldCount(); | ||
| RexExecutor executor = cluster.getPlanner().getExecutor(); | ||
| if (executor == null) { |
There was a problem hiding this comment.
+1, getExecutor is a nullable method.
| List<RelDataTypeField> fields = rowType.getFieldList(); | ||
| List<RexLiteral> constants = new ArrayList<>(); | ||
| RexBuilder builder = cluster.getRexBuilder(); | ||
| RexSimplify simplify = new RexSimplify(builder, RelOptPredicateList.EMPTY, executor); |
There was a problem hiding this comment.
Nit: Could the RexSimplify be passed as a parameter of this method? That would make the code simpler & cleaner, and we would avoid (re)creating a RexSimplify on each iteration that calls convertLiteralTypes.
… type Signed-off-by: Mihai Budiu <mbudiu@feldera.com>
|
The issue fixed here was first reported as part of https://issues.apache.org/jira/browse/CALCITE-6617