vlsi commented on a change in pull request #2272:
URL: https://github.com/apache/calcite/pull/2272#discussion_r528628858
##########
File path: core/src/main/java/org/apache/calcite/tools/RelBuilder.java
##########
@@ -495,6 +495,10 @@ public RexNode field(int inputCount, String alias, String
fieldName) {
/** Returns a reference to a given field of a record-valued expression. */
public RexNode field(RexNode e, String name) {
+ if (!e.getType().isStruct()) {
+ throw new IllegalArgumentException("Requested field " + name + " in
non-struct expression "
+ + e.toString() + " (type: " + e.getType() + ")");
+ }
Review comment:
Basically, I do not like to make `RexBuilder` aware of the ways
`getField` can fail.
In other words, it is not RexBuilder's business to tell if `non-sturcts` are
allowed to have fields or not.
I've had a very similar discussion with @julianhyde in
https://issues.apache.org/jira/browse/CALCITE-4217, and the takeaway was "don't
try to make sense of struct vs fieldList".
That is why I believe the necessary and sufficient solution here is:
1) Delegate the proper exception message to
`org.apache.calcite.rel.type.RelDataType#getField(String fieldName, ...)`
implementations. In other words, if someone implements `RelDataType`, then it
is their business to throw the appropriate exception.
2)
`org.apache.calcite.rex.RexBuilder#makeFieldAccess(org.apache.calcite.rex.RexNode,
int)` is slightly different since it can't delegate to `RelDataType`, and it
has to call `type.getFieldList()`. It could be left as is, and the type
implementation would throw "type ... has no fields" (which might be enough).
If you want to add clarification with the problematic `RexNode`, then you
could use
```java
try {
final List<RelDataTypeField> fields = type.getFieldList();
} catch (RuntimeException e) {
e.addSuppressed(new Throwable("Unable to get field " + i + " from rexNode
" + rexNode");
throw e
}
```
The important point is in both cases `RexBuilder` makes no assumptions on
`struct` vs `non-struct`.
----------------------------------------------------------------
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:
[email protected]
