alex-plekhanov commented on a change in pull request #9143:
URL: https://github.com/apache/ignite/pull/9143#discussion_r690144708
##########
File path:
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/schema/SchemaHolderImpl.java
##########
@@ -247,8 +248,11 @@ private static Object
affinityIdentity(CacheConfiguration<?, ?> ccfg) {
boolean descending = idxDesc.descending(idxField);
int fieldIdx = fieldDesc.fieldIndex();
- RelFieldCollation collation = new RelFieldCollation(fieldIdx,
- descending ? RelFieldCollation.Direction.DESCENDING :
RelFieldCollation.Direction.ASCENDING);
+ RelFieldCollation collation = new RelFieldCollation(
+ fieldIdx,
+ descending ? RelFieldCollation.Direction.DESCENDING :
RelFieldCollation.Direction.ASCENDING,
+ RelFieldCollation.NullDirection.FIRST
Review comment:
We have `NULLS LAST` for `DESC` order.
Also test for this should be added. For example, I've tested it with
`CalciteBasicSecondaryIndexIntegrationTest` class, added a new index `CITY
DESC`, put some nulls in `CITY` field and checked query `SELECT * FROM
Developer ORDER BY city DESC nulls first`: currently it returns index scan
without additional sort node and returns wrong nulls order.
Also, shouldn't we change our `defaultNullCollation` to `LOW` to fit our
indexes? Currently, if the user creates an index and then uses `order by` in a
query by indexed columns, by default index will not be used without nulls
first/nulls last clause. This can be confusing for some customers, perhaps
behavior for index and `ORDER BY` should be consistent. In the current H2
engine, we have nulls ordering policy similar to calcites `LOW`.
##########
File path:
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/metadata/IgniteMdCollation.java
##########
@@ -548,9 +548,9 @@ public int compare(List<RexLiteral> o1, List<RexLiteral>
o2) {
case FULL:
for (RelCollation collation : leftCollations) {
for (RelFieldCollation field :
collation.getFieldCollations()) {
- if (!(RelFieldCollation.NullDirection.LAST ==
field.nullDirection)) {
+ if
(!(RelFieldCollation.NullDirection.LAST.nullComparison ==
Review comment:
Looks like these class contains a lot of unused code, this method unused
too (applicable only for enumerable convention, but is not used even for
enumerable convention, instead methods from `RelMdCollation` used)
--
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]
