zabetak commented on code in PR #6315:
URL: https://github.com/apache/hive/pull/6315#discussion_r2822550948
##########
ql/src/test/queries/clientpositive/select_columns_subset_of_sort_by.q:
##########
@@ -0,0 +1,9 @@
+CREATE TABLE test (
+ col1 STRING,
+ col2 STRING
+);
+
+EXPLAIN CBO
+SELECT col1
+FROM test
+SORT BY col1, col2;
Review Comment:
The problem seems to affect also `HiveProjectSortTransposeRule` so using the
following query would also lead to the same exception:
```sql
set hive.optimize.limittranspose=true;
EXPLAIN CBO
SELECT col1
FROM test
ORDER BY col1, col2;
```
thus I would suggest to add this test case as well.
The two rules share some things in common so ideally there should be under
some common class but this can be done in a follow-up if time permits.
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveRelOptUtil.java:
##########
@@ -1107,7 +1107,15 @@ public static List<RelFieldCollation>
getNewRelFieldCollations(
Set<Integer> needed = new HashSet<>();
for (RelFieldCollation fc : sortCollation.getFieldCollations()) {
needed.add(fc.getFieldIndex());
- final RexNode node =
project.getProjects().get(map.getTarget(fc.getFieldIndex()));
+ int target;
+ try {
+ target = map.getTarget(fc.getFieldIndex());
+ } catch (Mappings.NoElementException e) {
+ // If there is no mapping for this field, we cannot push down the sort
+ LOG.error("Missing target mapping: {}", e.toString());
+ return null;
+ }
Review Comment:
I have the impression that we can avoid the exception handling logic here by
using the `Mappings.TargetMapping#getTargetOpt` API.
In general, whenever we craft try-catch blocks we should ask ourselves if
there is another way to achieve the same without relying on exceptions. For
details, check
"Item 69: Use exceptions only for exceptional conditions" in Effective Java
by Joshua Bloch.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
