[
https://issues.apache.org/jira/browse/PHOENIX-3452?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15649794#comment-15649794
]
chenglei edited comment on PHOENIX-3452 at 11/10/16 8:36 AM:
-------------------------------------------------------------
I think the problem is cause by the GroupByCompiler, when GroupBy.compile
method called the OrderPreservingTracker.track method to track the groupBy
expression's order,just as following(in GroupByCompiler.java):
{code:borderStyle=solid}
144 if (isOrderPreserving) {
145 OrderPreservingTracker tracker = new
OrderPreservingTracker(context, GroupBy.EMPTY_GROUP_BY, Ordering.UNORDERED,
expressions.size(), tupleProjector);
146 for (int i = 0; i < expressions.size(); i++) {
147 Expression expression = expressions.get(i);
148 tracker.track(expression);
149 }
{code}
The track method inappropriately used the sortOrder != SortOrder.getDefault()
as the thrid "isNullsLast" parameter as following(in
OrderPreservingTracker.java):
{code:borderStyle=solid}
101 public void track(Expression node) {
102 SortOrder sortOrder = node.getSortOrder();
103 track(node, sortOrder, sortOrder != SortOrder.getDefault());
104 }
105
106 public void track(Expression node, SortOrder sortOrder, boolean
isNullsLast) {
{code}
Once the node's SortOrder is SortOrder.DESC, the "isNullsLast" is true. it
affected the GroupBy 's isOrderPreserving as following(in
OrderPreservingTracker.java) :
{code:borderStyle=solid}
141 if (node.isNullable()) {
142 if (!Boolean.valueOf(isNullsLast).equals(isReverse)) {
143 isOrderPreserving = false;
144 isReverse = false;
145 return;
146 }
147 }
{code}
As far as the sql concerned, GroupBy 's "isOrderPreserving" property is
false,but the OrderByComilper thinks the OrderBy is
OrderBy.FWD_ROW_KEY_ORDER_BY. Because GroupByComilper 's "isOrderPreserving"
property is false,so the aggregated results returned by the RegionServer's
GroupedAggregateRegionObserver is unordered, and after the AggregatePlan of
client side get the aggregated results,the results just be sorted by the
aggregatedKey through MergeSortRowKeyResultIterator, so the final results is
not as expected.
Actually, the "isNullsLast" parameter is just related to orderBy ,it should
just affected the display order of "Null " in the sorted results , groupBy
should not be affetced by "isNullsLast". I wrote a simple unit test to
reproduce this problem in my patch:
{code:borderStyle=solid}
@Test
public void testGroupByDesc() throws Exception {
Connection conn = DriverManager.getConnection(getUrl());
try {
conn.createStatement().execute("DROP TABLE IF EXISTS
GROUPBYDESC_TEST");
String sql="CREATE TABLE IF NOT EXISTS GROUPBYDESC_TEST ( "+
"ORGANIZATION_ID VARCHAR,"+
"CONTAINER_ID VARCHAR,"+
"CONSTRAINT TEST_PK PRIMARY KEY ( "+
"ORGANIZATION_ID DESC,"+
"CONTAINER_ID DESC"+
"))";
conn.createStatement().execute(sql);
sql="SELECT ORGANIZATION_ID, CONTAINER_ID,count(*) FROM
GROUPBYDESC_TEST group by ORGANIZATION_ID, CONTAINER_ID";
PhoenixPreparedStatement statement =
conn.prepareStatement(sql).unwrap(PhoenixPreparedStatement.class);
QueryPlan queryPlan = statement.optimizeQuery(sql);
queryPlan.iterator();
assertTrue(queryPlan.getGroupBy().isOrderPreserving());
} finally {
conn.close();
}
}
{code}
I uploaded my patch, [~jamestaylor], please review.
was (Author: comnetwork):
I think the problem is cause by the GroupByCompiler, when GroupBy.compile
method called the OrderPreservingTracker.track method to track the groupBy
expression's order,just as following(in GroupByCompiler.java):
{code:borderStyle=solid}
144 if (isOrderPreserving) {
145 OrderPreservingTracker tracker = new
OrderPreservingTracker(context, GroupBy.EMPTY_GROUP_BY, Ordering.UNORDERED,
expressions.size(), tupleProjector);
146 for (int i = 0; i < expressions.size(); i++) {
147 Expression expression = expressions.get(i);
148 tracker.track(expression);
149 }
{code}
The track method inappropriately used the sortOrder != SortOrder.getDefault()
as the thrid "isNullsLast" parameter as following(in
OrderPreservingTracker.java):
{code:borderStyle=solid}
101 public void track(Expression node) {
102 SortOrder sortOrder = node.getSortOrder();
103 track(node, sortOrder, sortOrder != SortOrder.getDefault());
104 }
105
106 public void track(Expression node, SortOrder sortOrder, boolean
isNullsLast) {
{code}
Once the node's SortOrder is SortOrder.DESC, the "isNullsLast" is true. it
affected the GroupBy 's isOrderPreserving as following(in
OrderPreservingTracker.java) :
{code:borderStyle=solid}
141 if (node.isNullable()) {
142 if (!Boolean.valueOf(isNullsLast).equals(isReverse)) {
143 isOrderPreserving = false;
144 isReverse = false;
145 return;
146 }
147 }
{code}
Actually, the "isNullsLast" parameter is just related to orderBy ,it should
just affected the display order of "Null " in the sorted results , groupBy
should not be affetced by "isNullsLast". I wrote a simple unit test to
reproduce this problem in my patch:
{code:borderStyle=solid}
@Test
public void testGroupByDesc() throws Exception {
Connection conn = DriverManager.getConnection(getUrl());
try {
conn.createStatement().execute("DROP TABLE IF EXISTS
GROUPBYDESC_TEST");
String sql="CREATE TABLE IF NOT EXISTS GROUPBYDESC_TEST ( "+
"ORGANIZATION_ID VARCHAR,"+
"CONTAINER_ID VARCHAR,"+
"CONSTRAINT TEST_PK PRIMARY KEY ( "+
"ORGANIZATION_ID DESC,"+
"CONTAINER_ID DESC"+
"))";
conn.createStatement().execute(sql);
sql="SELECT ORGANIZATION_ID, CONTAINER_ID,count(*) FROM
GROUPBYDESC_TEST group by ORGANIZATION_ID, CONTAINER_ID";
PhoenixPreparedStatement statement =
conn.prepareStatement(sql).unwrap(PhoenixPreparedStatement.class);
QueryPlan queryPlan = statement.optimizeQuery(sql);
queryPlan.iterator();
assertTrue(queryPlan.getGroupBy().isOrderPreserving());
} finally {
conn.close();
}
}
{code}
I uploaded my patch, [~jamestaylor], please review.
> Secondary index and query using distinct: ORDER BY doesn't work correctly
> -------------------------------------------------------------------------
>
> Key: PHOENIX-3452
> URL: https://issues.apache.org/jira/browse/PHOENIX-3452
> Project: Phoenix
> Issue Type: Bug
> Affects Versions: 4.8.0
> Reporter: Joel Palmert
> Assignee: chenglei
> Attachments: PHOENIX-3452_v2.patch
>
>
> This may be related to PHOENIX-3451 but the behavior is different so filing
> it separately.
> Steps to repro:
> CREATE TABLE IF NOT EXISTS TEST.TEST (
> ORGANIZATION_ID CHAR(15) NOT NULL,
> CONTAINER_ID CHAR(15) NOT NULL,
> ENTITY_ID CHAR(15) NOT NULL,
> SCORE DOUBLE,
> CONSTRAINT TEST_PK PRIMARY KEY (
> ORGANIZATION_ID,
> CONTAINER_ID,
> ENTITY_ID
> )
> ) VERSIONS=1, MULTI_TENANT=TRUE, REPLICATION_SCOPE=1, TTL=31536000;
> CREATE INDEX IF NOT EXISTS TEST_SCORE ON TEST.TEST (CONTAINER_ID, SCORE DESC,
> ENTITY_ID DESC);
> UPSERT INTO test.test VALUES ('org1','container1','entityId6',1.1);
> UPSERT INTO test.test VALUES ('org1','container1','entityId5',1.2);
> UPSERT INTO test.test VALUES ('org1','container1','entityId4',1.3);
> UPSERT INTO test.test VALUES ('org1','container1','entityId3',1.4);
> UPSERT INTO test.test VALUES ('org1','container1','entityId2',1.5);
> UPSERT INTO test.test VALUES ('org1','container1','entityId1',1.6);
> SELECT DISTINCT entity_id, score
> FROM test.test
> WHERE organization_id = 'org1'
> AND container_id = 'container1'
> ORDER BY score DESC
> Notice that the returned results are not returned in descending score order.
> Instead they are returned in descending entity_id order. If I remove the
> DISTINCT or remove the secondary index the result is correct.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
