[ https://issues.apache.org/jira/browse/PHOENIX-6203?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17228475#comment-17228475 ]
Hadoop QA commented on PHOENIX-6203: ------------------------------------ | (x) *{color:red}-1 overall{color}* | \\ \\ || Vote || Subsystem || Runtime || Comment || | {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m 39s{color} | {color:blue} Docker mode activated. {color} | || || || || {color:brown} Prechecks {color} || | {color:green}+1{color} | {color:green} dupname {color} | {color:green} 0m 0s{color} | {color:green} No case conflicting files found. {color} | | {color:green}+1{color} | {color:green} hbaseanti {color} | {color:green} 0m 0s{color} | {color:green} Patch does not have any anti-patterns. {color} | | {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m 0s{color} | {color:green} The patch does not contain any @author tags. {color} | | {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m 0s{color} | {color:green} The patch appears to include 2 new or modified test files. {color} | || || || || {color:brown} master Compile Tests {color} || | {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 8m 20s{color} | {color:green} master passed {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 56s{color} | {color:green} master passed {color} | | {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 1m 34s{color} | {color:green} master passed {color} | | {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 43s{color} | {color:green} master passed {color} | | {color:blue}0{color} | {color:blue} spotbugs {color} | {color:blue} 3m 0s{color} | {color:blue} phoenix-core in master has 962 extant spotbugs warnings. {color} | || || || || {color:brown} Patch Compile Tests {color} || | {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 8m 53s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 1m 3s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} javac {color} | {color:green} 1m 3s{color} | {color:green} the patch passed {color} | | {color:red}-1{color} | {color:red} checkstyle {color} | {color:red} 1m 33s{color} | {color:red} phoenix-core: The patch generated 37 new + 3027 unchanged - 33 fixed = 3064 total (was 3060) {color} | | {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m 0s{color} | {color:green} The patch has no whitespace issues. {color} | | {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 48s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} spotbugs {color} | {color:green} 3m 38s{color} | {color:green} the patch passed {color} | || || || || {color:brown} Other Tests {color} || | {color:red}-1{color} | {color:red} unit {color} | {color:red}106m 47s{color} | {color:red} phoenix-core in the patch failed. {color} | | {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m 25s{color} | {color:green} The patch does not generate ASF License warnings. {color} | | {color:black}{color} | {color:black} {color} | {color:black}138m 59s{color} | {color:black} {color} | \\ \\ || Reason || Tests || | Failed junit tests | phoenix.end2end.OrphanViewToolIT | \\ \\ || Subsystem || Report/Notes || | Docker | ClientAPI=1.40 ServerAPI=1.40 base: https://ci-hadoop.apache.org/job/PreCommit-PHOENIX-Build/172/artifact/patchprocess/Dockerfile | | JIRA Issue | PHOENIX-6203 | | JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/13014901/PHOENIX-6203.master.000.patch | | Optional Tests | dupname asflicense javac javadoc unit spotbugs hbaseanti checkstyle compile | | uname | Linux 313e9aadfc6d 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/PreCommit-PHOENIX-Build/172/artifact/patchprocess/diff-checkstyle-phoenix-core.txt | | unit | https://ci-hadoop.apache.org/job/PreCommit-PHOENIX-Build/172/artifact/patchprocess/patch-unit-phoenix-core.txt | | Test Results | https://ci-hadoop.apache.org/job/PreCommit-PHOENIX-Build/172/testReport/ | | Max. process+thread count | 7025 (vs. ulimit of 30000) | | modules | C: phoenix-core U: phoenix-core | | Console output | https://ci-hadoop.apache.org/job/PreCommit-PHOENIX-Build/172/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. > 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)