PHOENIX-6400 Do no use local index with uncovered columns in the WHERE clause. by lhofhansl · Pull Request #1158 · apache/phoenix

lhofhansl · 2021-03-03T21:00:47Z

See long discussion on Jira: https://issues.apache.org/jira/browse/PHOENIX-6400

kadirozde · 2021-03-03T23:34:44Z

+ && (table.getIndexType() == null || table.getIndexType() == IndexType.GLOBAL)) {
+ String schemaNameStr = table.getSchemaName()==null?null:table.getSchemaName().getString();
+ String tableNameStr = table.getTableName()==null?null:table.getTableName().getString();
+ throw new ColumnNotFoundException(schemaNameStr, tableNameStr, null, ref.getColumn().getName().getString());


It is not clear how returning ColumnNotFoundException results in skipping a plan. Can we add some comments here for that?

I just brought that code back from before PHOENIX-5109.
It throws an exception, which is then caught in query enumeration in the optimizing phase and consequently ignores that plan.
Happy to add a comment to that extent.

Something like, "This exception will be caught in query enumeration in the optimizing phase and consequently the plan will be ignored" as you wrote, will be helpful. I will then approve it. Thanks!

How about this?

stoty · 2021-03-03T23:54:47Z

💔 -1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	7m 43s	Docker mode activated.
		_ Prechecks _
+1 💚	dupname	0m 0s	No case conflicting files found.
+1 💚	hbaseanti	0m 0s	Patch does not have any anti-patterns.
+1 💚	@author	0m 0s	The patch does not contain any @author tags.
-1 ❌	test4tests	0m 0s	The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
		_ master Compile Tests _
+1 💚	mvninstall	17m 9s	master passed
+0	hbaserecompile	25m 12s	HBase recompiled.
+1 💚	compile	1m 4s	master passed
+1 💚	checkstyle	0m 34s	master passed
+1 💚	javadoc	0m 56s	master passed
+0 🆗	spotbugs	3m 18s	phoenix-core in master has 959 extant spotbugs warnings.
		_ Patch Compile Tests _
+1 💚	mvninstall	9m 45s	the patch passed
+0	hbaserecompile	20m 4s	HBase recompiled.
+1 💚	compile	1m 5s	the patch passed
+1 💚	javac	1m 5s	the patch passed
-1 ❌	checkstyle	0m 35s	phoenix-core: The patch generated 19 new + 155 unchanged - 0 fixed = 174 total (was 155)
+1 💚	whitespace	0m 0s	The patch has no whitespace issues.
+1 💚	javadoc	0m 51s	the patch passed
+1 💚	spotbugs	3m 29s	the patch passed
		_ Other Tests _
+1 💚	unit	110m 26s	phoenix-core in the patch passed.
+1 💚	asflicense	0m 15s	The patch does not generate ASF License warnings.
		173m 28s

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1158/1/artifact/yetus-general-check/output/Dockerfile
GITHUB PR	#1158
Optional Tests	dupname asflicense javac javadoc unit spotbugs hbaserebuild hbaseanti checkstyle compile
uname	Linux 18ca91ffdb62 4.15.0-101-generic #102-Ubuntu SMP Mon May 11 10:07:26 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev/phoenix-personality.sh
git revision	master / `f7d25b9`
Default Java	Private Build-1.8.0_242-8u242-b08-0ubuntu3~16.04-b08
checkstyle	https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1158/1/artifact/yetus-general-check/output/diff-checkstyle-phoenix-core.txt
Test Results	https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1158/1/testReport/
Max. process+thread count	9689 (vs. ulimit of 30000)
modules	C: phoenix-core U: phoenix-core
Console output	https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1158/1/console
versions	git=2.7.4 maven=3.3.9 spotbugs=4.1.3
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

…E clause.

kadirozde

+1, Thanks!

kadirozde · 2021-03-04T01:34:39Z

+ && (table.getIndexType() == null || table.getIndexType() == IndexType.GLOBAL)) {
+ String schemaNameStr = table.getSchemaName()==null?null:table.getSchemaName().getString();
+ String tableNameStr = table.getTableName()==null?null:table.getTableName().getString();
+ throw new ColumnNotFoundException(schemaNameStr, tableNameStr, null, ref.getColumn().getName().getString());


stoty · 2021-03-04T03:50:10Z

💔 -1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	1m 43s	Docker mode activated.
		_ Prechecks _
+1 💚	dupname	0m 0s	No case conflicting files found.
+1 💚	hbaseanti	0m 0s	Patch does not have any anti-patterns.
+1 💚	@author	0m 0s	The patch does not contain any @author tags.
-1 ❌	test4tests	0m 0s	The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
		_ master Compile Tests _
+1 💚	mvninstall	17m 14s	master passed
+0	hbaserecompile	25m 25s	HBase recompiled.
+1 💚	compile	1m 6s	master passed
+1 💚	checkstyle	0m 36s	master passed
+1 💚	javadoc	0m 56s	master passed
+0 🆗	spotbugs	3m 18s	phoenix-core in master has 959 extant spotbugs warnings.
		_ Patch Compile Tests _
+1 💚	mvninstall	9m 59s	the patch passed
+0	hbaserecompile	21m 12s	HBase recompiled.
+1 💚	compile	1m 5s	the patch passed
+1 💚	javac	1m 5s	the patch passed
-1 ❌	checkstyle	0m 35s	phoenix-core: The patch generated 21 new + 154 unchanged - 1 fixed = 175 total (was 155)
+1 💚	whitespace	0m 0s	The patch has no whitespace issues.
+1 💚	javadoc	0m 50s	the patch passed
+1 💚	spotbugs	3m 32s	the patch passed
		_ Other Tests _
-1 ❌	unit	107m 55s	phoenix-core in the patch failed.
+1 💚	asflicense	0m 45s	The patch does not generate ASF License warnings.
		168m 49s

Reason	Tests
Failed junit tests	phoenix.end2end.PermissionsCacheIT

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1158/2/artifact/yetus-general-check/output/Dockerfile
GITHUB PR	#1158
Optional Tests	dupname asflicense javac javadoc unit spotbugs hbaserebuild hbaseanti checkstyle compile
uname	Linux 080ad0c5ed02 4.15.0-101-generic #102-Ubuntu SMP Mon May 11 10:07:26 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev/phoenix-personality.sh
git revision	master / `f7d25b9`
Default Java	Private Build-1.8.0_242-8u242-b08-0ubuntu3~16.04-b08
checkstyle	https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1158/2/artifact/yetus-general-check/output/diff-checkstyle-phoenix-core.txt
unit	https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1158/2/artifact/yetus-general-check/output/patch-unit-phoenix-core.txt
Test Results	https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1158/2/testReport/
Max. process+thread count	9706 (vs. ulimit of 30000)
modules	C: phoenix-core U: phoenix-core
Console output	https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1158/2/console
versions	git=2.7.4 maven=3.3.9 spotbugs=4.1.3
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

stoty · 2021-03-04T06:36:41Z

💔 -1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	1m 28s	Docker mode activated.
		_ Prechecks _
+1 💚	dupname	0m 0s	No case conflicting files found.
+1 💚	hbaseanti	0m 0s	Patch does not have any anti-patterns.
+1 💚	@author	0m 0s	The patch does not contain any @author tags.
-1 ❌	test4tests	0m 0s	The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
		_ master Compile Tests _
+1 💚	mvninstall	17m 11s	master passed
+0	hbaserecompile	24m 19s	HBase recompiled.
+1 💚	compile	1m 7s	master passed
+1 💚	checkstyle	0m 35s	master passed
+1 💚	javadoc	0m 55s	master passed
+0 🆗	spotbugs	3m 24s	phoenix-core in master has 959 extant spotbugs warnings.
		_ Patch Compile Tests _
+1 💚	mvninstall	9m 59s	the patch passed
+0	hbaserecompile	20m 21s	HBase recompiled.
+1 💚	compile	1m 5s	the patch passed
+1 💚	javac	1m 5s	the patch passed
-1 ❌	checkstyle	0m 35s	phoenix-core: The patch generated 21 new + 154 unchanged - 1 fixed = 175 total (was 155)
+1 💚	whitespace	0m 0s	The patch has no whitespace issues.
+1 💚	javadoc	0m 50s	the patch passed
+1 💚	spotbugs	3m 32s	the patch passed
		_ Other Tests _
+1 💚	unit	108m 11s	phoenix-core in the patch passed.
+1 💚	asflicense	0m 14s	The patch does not generate ASF License warnings.
		164m 29s

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1158/3/artifact/yetus-general-check/output/Dockerfile
GITHUB PR	#1158
Optional Tests	dupname asflicense javac javadoc unit spotbugs hbaserebuild hbaseanti checkstyle compile
uname	Linux ed3310a10b1a 4.15.0-101-generic #102-Ubuntu SMP Mon May 11 10:07:26 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev/phoenix-personality.sh
git revision	master / `f7d25b9`
Default Java	Private Build-1.8.0_242-8u242-b08-0ubuntu3~16.04-b08
checkstyle	https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1158/3/artifact/yetus-general-check/output/diff-checkstyle-phoenix-core.txt
Test Results	https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1158/3/testReport/
Max. process+thread count	10719 (vs. ulimit of 30000)
modules	C: phoenix-core U: phoenix-core
Console output	https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1158/3/console
versions	git=2.7.4 maven=3.3.9 spotbugs=4.1.3
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

lhofhansl requested a review from kadirozde March 3, 2021 22:08

kadirozde reviewed Mar 3, 2021
View reviewed changes

lhofhansl force-pushed the 6400 branch 2 times, most recently from 48404a9 to 92af25dCompare March 4, 2021 01:01

PHOENIX-6400 Do no use local index with uncovered columns in the WHER…
92af25d
…E clause.

kadirozde approved these changes Mar 4, 2021
View reviewed changes

lhofhansl merged commit d161867 into apache:masterMar 4, 2021

Navigation Menu

Uh oh!

PHOENIX-6400 Do no use local index with uncovered columns in the WHERE clause. - #1158

PHOENIX-6400 Do no use local index with uncovered columns in the WHERE clause.#1158
lhofhansl merged 1 commit into
apache:masterfrom
lhofhansl:6400

lhofhansl commented Mar 3, 2021

Uh oh!

kadirozdeMar 3, 2021

Uh oh!

lhofhanslMar 3, 2021

Uh oh!

kadirozdeMar 3, 2021

Uh oh!

lhofhanslMar 4, 2021

Uh oh!

kadirozdeMar 4, 2021

Uh oh!

stoty commented Mar 3, 2021

Uh oh!

kadirozde left a comment

Uh oh!

kadirozdeMar 4, 2021

Uh oh!

stoty commented Mar 4, 2021

Uh oh!

stoty commented Mar 4, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

lhofhansl commented Mar 3, 2021

Uh oh!

kadirozdeMar 3, 2021

Choose a reason for hiding this comment

Uh oh!

lhofhanslMar 3, 2021

Choose a reason for hiding this comment

Uh oh!

kadirozdeMar 3, 2021

Choose a reason for hiding this comment

Uh oh!

lhofhanslMar 4, 2021

Choose a reason for hiding this comment

Uh oh!

kadirozdeMar 4, 2021

Choose a reason for hiding this comment

Uh oh!

stoty commented Mar 3, 2021

Uh oh!

kadirozde left a comment

Choose a reason for hiding this comment

Uh oh!

kadirozdeMar 4, 2021

Choose a reason for hiding this comment

Uh oh!

stoty commented Mar 4, 2021

Uh oh!

stoty commented Mar 4, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants