rubenada commented on a change in pull request #2272:
URL: https://github.com/apache/calcite/pull/2272#discussion_r528621824
##########
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:
@vlsi if I am not mistaken,
`org.apache.calcite.piglet.DynamicTupleRecordType` inherits `isStruct` from
`org.apache.calcite.rel.type.DynamicRecordTypeImpl`:
```
@Override public boolean isStruct() {
return true;
}
```
But, what you say is a valid point, if we have the assumption that a
non-struct type can support getField, then `isStruct()` check must not be
added. I'm not sure if this scenario makes sense at all, but just in case
perhaps the most conservative approach would be removing the `isStruct()`
check and just rely on the `AssertionError` thrown by `RelDataTypeImpl`.
Honestly, this is a minor change, I am open to any suggestion.
----------------------------------------------------------------
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]
