[
https://issues.apache.org/jira/browse/PHOENIX-2194?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14735359#comment-14735359
]
Samarth Jain commented on PHOENIX-2194:
---------------------------------------
Patch looks good to me, [~jamestaylor]. Just one comment:
In SkipScanFilter.java, I see that you are using asserts to make sure certain
invariants are met:
{code}
+ assert ( previousCellHint == null
|| Bytes.compareTo(nextCellHint.getRowArray(),
nextCellHint.getRowOffset(),
nextCellHint.getRowLength(),
previousCellHint.getRowArray(), previousCellHint
- .getRowOffset(), previousCellHint.getRowLength())
> 0 : "next hint must come after previous hint (prev="
- + previousCellHint + ", next=" + nextCellHint + ", kv=" + kv +
")";
+ .getRowOffset(), previousCellHint.getRowLength())
> 0 ) : "next hint must come after previous hint (prev=" + previousCellHint +
", next=" + nextCellHint + ", kv=" + kv + ")";
{code}
java.lang.assert, in general, is not enabled by default. Sometimes we have to
turn them on in IDEs too. So one can not always be sure while running tests or
when running code in production if the asserts are really being used. An
alternative is to use PreConditions.checkState() or checkArgument().
> order by should not require all PK fields with = constraint
> -----------------------------------------------------------
>
> Key: PHOENIX-2194
> URL: https://issues.apache.org/jira/browse/PHOENIX-2194
> Project: Phoenix
> Issue Type: Bug
> Affects Versions: 4.5.0
> Environment: linux
> Reporter: Gary Horen
> Assignee: James Taylor
> Labels: AtMention, SFDC
> Fix For: 4.6, 4.5.2
>
> Attachments: PHOENIX-2194-tests.patch, PHOENIX-2194-tests2.patch,
> PHOENIX-2194.patch, PHOENIX-2194_master.patch, PHOENIX-2194_master.patch,
> PHOENIX-2194_v2.patch, PHOENIX-2194_v3.patch, PHOENIX-2194_v4.patch,
> PHOENIX-2194_v5.patch, PHOENIX-2194_v6.patch
>
>
> Here is a table:
> CREATE TABLE IF NOT EXISTS FEEDS.STUFF
> (
> STUFF CHAR(15) NOT NULL,
> NONSENSE CHAR(15) NOT NULL
> CONSTRAINT PK PRIMARY KEY
> (
> STUFF,
> NONSENSE
>
> )
> ) VERSIONS=1,MULTI_TENANT=TRUE,REPLICATION_SCOPE=1
> Here is a query:
> explain SELECT * FROM feeds.stuff
> where stuff = ' '
> and nonsense > ' '
> order by nonsense
> Here is the plan:
> CLIENT 1-CHUNK PARALLEL 1-WAY RANGE SCAN
> SERVER FILTER BY FIRST KEY ONLY
> SERVER TOP 100 ROWS SORTED BY [NONSE
> CLIENT MERGE SORT
> If I change to ORDER BY STUFF, NONSENSE I get:
> CLIENT 1-CHUNK SERIAL 1-WAY RANGE SCAN O
> SERVER FILTER BY FIRST KEY ONLY AND
> SERVER 100 ROW LIMIT
> CLIENT 100 ROW LIMIT
> Since the leading constraint is =, ORDER BY will be unaffected by it, so
> ORDER BY should not need the leading constraint; it should only require the
> columns whose values would vary (which, since they are ordered by the key,
> should (and do) result in the client side sort being optimized out.) Having
> to include the leading = constraints in the ORDER BY clause is very
> counter-intuitive.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
