[
https://issues.apache.org/jira/browse/PHOENIX-6203?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17228480#comment-17228480
]
ASF GitHub Bot commented on PHOENIX-6203:
-----------------------------------------
stoty commented on pull request #957:
URL: https://github.com/apache/phoenix/pull/957#issuecomment-723911111
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 8s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files
found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any
anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any
@author tags. |
| +1 :green_heart: | test4tests | 0m 0s | The patch appears to include
2 new or modified test files. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 14m 18s | master passed |
| +1 :green_heart: | compile | 1m 2s | master passed |
| +1 :green_heart: | checkstyle | 1m 20s | master passed |
| +1 :green_heart: | javadoc | 0m 45s | master passed |
| +0 :ok: | spotbugs | 3m 11s | phoenix-core in master has 962 extant
spotbugs warnings. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 9m 2s | the patch passed |
| +1 :green_heart: | compile | 0m 57s | the patch passed |
| +1 :green_heart: | javac | 0m 57s | the patch passed |
| -1 :x: | checkstyle | 1m 20s | phoenix-core: The patch generated 6
new + 3059 unchanged - 2 fixed = 3065 total (was 3061) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace
issues. |
| +1 :green_heart: | javadoc | 0m 45s | the patch passed |
| +1 :green_heart: | spotbugs | 3m 18s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 159m 44s | phoenix-core in the patch failed. |
| +1 :green_heart: | asflicense | 0m 23s | The patch does not generate
ASF License warnings. |
| | | 199m 50s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.40 ServerAPI=1.40 base:
https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-957/3/artifact/yetus-general-check/output/Dockerfile
|
| GITHUB PR | https://github.com/apache/phoenix/pull/957 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs hbaseanti
checkstyle compile |
| uname | Linux 62041d9e2d6e 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9
23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev/phoenix-personality.sh |
| git revision | master / 859b16c |
| 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-957/3/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-957/3/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-957/3/testReport/
|
| Max. process+thread count | 6199 (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-957/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.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> CQS.getTable(byte[] tableName) does not throw TNFE even if table doesn't exist
> ------------------------------------------------------------------------------
>
> Key: PHOENIX-6203
> URL: https://issues.apache.org/jira/browse/PHOENIX-6203
> Project: Phoenix
> Issue Type: Bug
> Affects Versions: 5.0.0, 4.15.0
> Reporter: Viraj Jasani
> Assignee: Viraj Jasani
> Priority: Major
> Fix For: 5.1.0, 4.16.0
>
> Attachments: PHOENIX-6203.4.x.000.patch, PHOENIX-6203.master.000.patch
>
>
> CQSI.getTable(byte[] tableName)
> ([here|https://github.com/apache/phoenix/blob/master/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java#L481])
> is being used at multiple places to retrieve Table object from Connection
> which is opened by CQSI.
> However, getTable() seems to be throwing TNFE with the expectation that
> underlying Connection.getTable(TableName tableName) will throw
> org.apache.hadoop.hbase.TNFE , which never happens because
> Connection.getTable() only returns Table implementation to access table,
> however, it does not take into consideration whether given table exists.
> Admin.tableExists() is the only reliable API to determine whether given table
> exists.
> One simple test to ensure the above finding holds true with current
> CQSI.getTable() method.
> {code:java}
> @Test
> public void test1() throws Exception {
> try (Connection conn = DriverManager.getConnection(getJdbcUrl())) {
> String tableName = SchemaUtil.getTableName(generateUniqueName(),
> generateUniqueName());
> // create parent table
> String ddl = "CREATE TABLE " + tableName
> + " (col1 INTEGER NOT NULL, col2 INTEGER " + "CONSTRAINT pk PRIMARY
> KEY (col1))";
> conn.createStatement().execute(ddl);
> ConnectionQueryServices
> cqs=conn.unwrap(PhoenixConnection.class).getQueryServices();
> // table does exist and cqs.getTable() does not throw TNFE
> Table table1 = cqs.getTable(Bytes.toBytes(tableName));
> assertNotNull(table1);
> // this is correct check for existence of table
> assertTrue(cqs.getAdmin().tableExists(TableName.valueOf(tableName)));
> conn.createStatement().execute("DROP TABLE " + tableName);
> // table has been dropped i.e does not exist, still cqs.getTable()
> does not throw TNFE
> Table table2 = cqs.getTable(Bytes.toBytes(tableName));
> assertNotNull(table2);
> assertEquals(table1.getName(), table2.getName());
> // this is correct check for existence of table
> assertFalse(cqs.getAdmin().tableExists(TableName.valueOf(tableName)));
> // table never existed, still cqs.getTable() does not throw TNFE
> Table table3 = cqs.getTable(Bytes.toBytes("abc"));
> assertNotNull(table3);
> assertEquals(table3.getName(), TableName.valueOf("abc"));
> // this is correct check for existence of table
> assertFalse(cqs.getAdmin().tableExists(TableName.valueOf("abc")));
> }
> }
> {code}
>
> Since CQS.getTable() is basic utility, it is being used at many places. We
> might have to prefer one of these solutions:
> # We change CQSI.getTable() to actually use Admin.tableExists() and
> accordingly either return Table object from Connection or throw TNFE
> # Identify all callers of CQSI.getTable() and determine how many of them
> actually needs to know whether table exist and accordingly use
> CQSI.getTable(), (maybe by passing a flag) and catch TNFE in all such callers
> (which they might already be doing with expectation of catching TNFE cases).
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
