[
https://issues.apache.org/jira/browse/PHOENIX-2171?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14662286#comment-14662286
]
Samarth Jain commented on PHOENIX-2171:
---------------------------------------
Patch looks good [~jamestaylor]. Some minor comments:
1) In PDataTypeTest add a test like testDoubleComparison() for float too since
it looks like you have changed the logic in decodeFloat() too. Maybe even
better if you could generalize the comparison test for all data types and sort
orders. Also, it would be good to add tests with arrays too that have different
sort orders.
2) Would it make sense to confirm that the query being executed in
testSkipScanCompare is actually using the skip scan filter?
3) In testNonPkCompare(), a couple more tests with an order by could probably
be added like this:
{code}
+ @Test
+ public void testNonPKCompare() throws Exception {
+ List<Integer> expectedResults = Lists.newArrayList(2,3,4);
+ Integer[] saltBuckets = new Integer[] {null,3};
+ PDataType[] dataTypes = new PDataType[] {PDecimal.INSTANCE,
PDouble.INSTANCE, PFloat.INSTANCE};
+ for (Integer saltBucket : saltBuckets) {
+ for (PDataType dataType : dataTypes) {
+ for (SortOrder sortOrder : SortOrder.values()) {
+ testCompareCompositeKey(saltBucket, dataType, sortOrder,
"", expectedResults, "");
testCompareCompositeKey(saltBucket, dataType, sortOrder,
"", expectedResults, ""ORDER BY k1 DESC"");
testCompareCompositeKey(saltBucket, dataType, sortOrder,
"", expectedResults, ""ORDER BY k2 ASC"");
+ }
+ }
+ }
+ }
{code}
4) Also tests that try to ORDER BY ASC when the column is declared with sort
order DESC.
5) In SortOrderIT#testCompareCompositeKey remove the System.out call and
commented code.
{code}
+ System.out.println(rs.getInt(1));
+// assertTrue (tableName, rs.next());
+// assertEquals(tableName, k,rs.getInt(1));
{code}
> DOUBLE and FLOAT DESC are stored as ASC
> ---------------------------------------
>
> Key: PHOENIX-2171
> URL: https://issues.apache.org/jira/browse/PHOENIX-2171
> Project: Phoenix
> Issue Type: Bug
> Reporter: James Taylor
> Priority: Critical
> Attachments: PHOENIX-2171.patch
>
>
> Our PDouble.getCodec().decodeDouble() and PFloat.getCodec().decodeFloat()
> methods are updating the byte array in-place when the data is DESC which is a
> big no-no as it essentially corrupts data. The end effect is that data which
> is attempted to be stored as DESC is stored ASC instead, with the data
> appearing as being corruprt. Not sure if this is always the case for ingest
> paths, but a common UPSERT VALUES is impacted:
> {code}
> 0: jdbc:phoenix:localhost> create table dd (k double primary key desc);
> No rows affected (1.356 seconds)
> 0: jdbc:phoenix:localhost> upsert into dd values (1.0);
> 1 row affected (0.054 seconds)
> 0: jdbc:phoenix:localhost> upsert into dd values (2.0);
> 1 row affected (0.005 seconds)
> 0: jdbc:phoenix:localhost> select * from dd;
> +------------------------------------------+
> | K |
> +------------------------------------------+
> | -1.0000000000000004 |
> | -2.000000000000001 |
> +------------------------------------------+
> 2 rows selected (0.038 seconds)
> {code}
> Not sure how to fix this in terms of data that has already been written. One
> potential solution is to switch the column to be ASC instead of DESC (since
> that's how it is actually stored):
> {code}
> put 'SYSTEM.CATALOG', "\x00\x00DD\x00K", '0:SORT_ORDER', "\x80\x00\x00\x00"
> {code}
> And now the data is interpreted correctly:
> {code}
> 0: jdbc:phoenix:localhost> select * from dd;
> +------------------------------------------+
> | K |
> +------------------------------------------+
> | 1.0 |
> | 2.0 |
> +------------------------------------------+
> 2 rows selected (6.157 seconds)
> {code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
