InvisibleProgrammer commented on code in PR #4384: URL: https://github.com/apache/hive/pull/4384#discussion_r1226514750
########## standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/entitymappers/CompactionInfoMapper.java: ########## @@ -0,0 +1,135 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hive.metastore.txn.entitymappers; + +import org.apache.hadoop.hive.metastore.api.MetaException; +import org.apache.hadoop.hive.metastore.txn.CompactionInfo; +import org.apache.hadoop.hive.metastore.txn.MetaWrapperException; +import org.apache.hadoop.hive.metastore.txn.TxnUtils; +import org.springframework.jdbc.core.RowMapper; +import org.springframework.jdbc.core.namedparam.MapSqlParameterSource; +import org.springframework.jdbc.core.namedparam.SqlParameterSource; + +import java.sql.ResultSet; +import java.sql.SQLException; + +public class CompactionInfoMapper implements RowMapper<CompactionInfo>, SqlParamSourceMapper<CompactionInfo> { + + private long compactionEndTime = 0L; + + // language=PostgreSQL + public static final String SELECT = + "SELECT \"CQ_ID\", \"CQ_DATABASE\", \"CQ_TABLE\", \"CQ_PARTITION\", " + + "\"CQ_STATE\", \"CQ_TYPE\", \"CQ_TBLPROPERTIES\", \"CQ_WORKER_ID\", \"CQ_START\", \"CQ_RUN_AS\", " + + "\"CQ_HIGHEST_WRITE_ID\", \"CQ_META_INFO\", \"CQ_HADOOP_JOB_ID\", \"CQ_ERROR_MESSAGE\", " + + "\"CQ_ENQUEUE_TIME\", \"CQ_WORKER_VERSION\", \"CQ_INITIATOR_ID\", \"CQ_INITIATOR_VERSION\", " + + "\"CQ_RETRY_RETENTION\", \"CQ_NEXT_TXN_ID\", \"CQ_TXN_ID\", \"CQ_COMMIT_TIME\", \"CQ_POOL_NAME\", " + + "\"CQ_NUMBER_OF_BUCKETS\", \"CQ_ORDER_BY\" FROM \"COMPACTION_QUEUE\" WHERE \"CQ_ID\" = :id"; + + // language=PostgreSQL + public static final String INSERT = + "INSERT INTO \"COMPLETED_COMPACTIONS\" " + + " (\"CC_ID\", \"CC_DATABASE\", \"CC_TABLE\", \"CC_PARTITION\", \"CC_STATE\", \"CC_TYPE\", " + + " \"CC_TBLPROPERTIES\", \"CC_WORKER_ID\", \"CC_START\", \"CC_END\", \"CC_RUN_AS\", " + + " \"CC_HIGHEST_WRITE_ID\", \"CC_META_INFO\", \"CC_HADOOP_JOB_ID\", \"CC_ERROR_MESSAGE\", " + + " \"CC_ENQUEUE_TIME\", \"CC_WORKER_VERSION\", \"CC_INITIATOR_ID\", \"CC_INITIATOR_VERSION\"," + + " \"CC_NEXT_TXN_ID\", \"CC_TXN_ID\", \"CC_COMMIT_TIME\", \"CC_POOL_NAME\", \"CC_NUMBER_OF_BUCKETS\", " + + " \"CC_ORDER_BY\") " + + " VALUES(:id,:dbname,:tableName,:partName,:state,:type,:properties,:workerId,:start,:endTime," + + " :runAs,:highestWriteId,:metaInfo,:hadoopJobId,:errorMessage,:enqueueTime,:workerVersion,:initiatorId," + + " :initiatorVersion,:nextTxnId,:txnId,:commitTime,:poolName,:numberOfBuckets,:orderByClause)"; + + + public long getCompactionEndTime() { + return compactionEndTime; + } + + public CompactionInfoMapper setCompactionEndTime(long compactionEndTime) { + this.compactionEndTime = compactionEndTime; + return this; + } + + @Override + public CompactionInfo mapRow(ResultSet rs, int rowNum) throws SQLException { + CompactionInfo fullCi = new CompactionInfo(); + fullCi.id = rs.getLong(1); + fullCi.dbname = rs.getString(2); + fullCi.tableName = rs.getString(3); + fullCi.partName = rs.getString(4); + fullCi.state = rs.getString(5).charAt(0);//cq_state + try { Review Comment: Can that kind of extra work with error handling stuff extracted into a private method to make the code easier to read? ########## standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/entitymappers/CompactionInfoMapper.java: ########## @@ -0,0 +1,135 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hive.metastore.txn.entitymappers; + +import org.apache.hadoop.hive.metastore.api.MetaException; +import org.apache.hadoop.hive.metastore.txn.CompactionInfo; +import org.apache.hadoop.hive.metastore.txn.MetaWrapperException; +import org.apache.hadoop.hive.metastore.txn.TxnUtils; +import org.springframework.jdbc.core.RowMapper; +import org.springframework.jdbc.core.namedparam.MapSqlParameterSource; +import org.springframework.jdbc.core.namedparam.SqlParameterSource; + +import java.sql.ResultSet; +import java.sql.SQLException; + +public class CompactionInfoMapper implements RowMapper<CompactionInfo>, SqlParamSourceMapper<CompactionInfo> { + + private long compactionEndTime = 0L; + + // language=PostgreSQL + public static final String SELECT = + "SELECT \"CQ_ID\", \"CQ_DATABASE\", \"CQ_TABLE\", \"CQ_PARTITION\", " + + "\"CQ_STATE\", \"CQ_TYPE\", \"CQ_TBLPROPERTIES\", \"CQ_WORKER_ID\", \"CQ_START\", \"CQ_RUN_AS\", " + + "\"CQ_HIGHEST_WRITE_ID\", \"CQ_META_INFO\", \"CQ_HADOOP_JOB_ID\", \"CQ_ERROR_MESSAGE\", " + + "\"CQ_ENQUEUE_TIME\", \"CQ_WORKER_VERSION\", \"CQ_INITIATOR_ID\", \"CQ_INITIATOR_VERSION\", " + + "\"CQ_RETRY_RETENTION\", \"CQ_NEXT_TXN_ID\", \"CQ_TXN_ID\", \"CQ_COMMIT_TIME\", \"CQ_POOL_NAME\", " + + "\"CQ_NUMBER_OF_BUCKETS\", \"CQ_ORDER_BY\" FROM \"COMPACTION_QUEUE\" WHERE \"CQ_ID\" = :id"; + + // language=PostgreSQL + public static final String INSERT = + "INSERT INTO \"COMPLETED_COMPACTIONS\" " + + " (\"CC_ID\", \"CC_DATABASE\", \"CC_TABLE\", \"CC_PARTITION\", \"CC_STATE\", \"CC_TYPE\", " + + " \"CC_TBLPROPERTIES\", \"CC_WORKER_ID\", \"CC_START\", \"CC_END\", \"CC_RUN_AS\", " + + " \"CC_HIGHEST_WRITE_ID\", \"CC_META_INFO\", \"CC_HADOOP_JOB_ID\", \"CC_ERROR_MESSAGE\", " + + " \"CC_ENQUEUE_TIME\", \"CC_WORKER_VERSION\", \"CC_INITIATOR_ID\", \"CC_INITIATOR_VERSION\"," + + " \"CC_NEXT_TXN_ID\", \"CC_TXN_ID\", \"CC_COMMIT_TIME\", \"CC_POOL_NAME\", \"CC_NUMBER_OF_BUCKETS\", " + + " \"CC_ORDER_BY\") " + + " VALUES(:id,:dbname,:tableName,:partName,:state,:type,:properties,:workerId,:start,:endTime," + + " :runAs,:highestWriteId,:metaInfo,:hadoopJobId,:errorMessage,:enqueueTime,:workerVersion,:initiatorId," + + " :initiatorVersion,:nextTxnId,:txnId,:commitTime,:poolName,:numberOfBuckets,:orderByClause)"; + + + public long getCompactionEndTime() { + return compactionEndTime; + } + + public CompactionInfoMapper setCompactionEndTime(long compactionEndTime) { + this.compactionEndTime = compactionEndTime; + return this; + } + + @Override + public CompactionInfo mapRow(ResultSet rs, int rowNum) throws SQLException { + CompactionInfo fullCi = new CompactionInfo(); + fullCi.id = rs.getLong(1); Review Comment: I think it is really hard to understand when we read row data from resultset by column position indexes. Instead of that, I recommend using column naming, like `fullCi.id = rs.getLong("id");`. With column indexes, you can easily make a mistake by referencing wrong column. And also, changing the resultset, like adding a new third column and sliding the others right, can be tedious. ########## standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/entitymappers/CompactionInfoMapper.java: ########## @@ -0,0 +1,135 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hive.metastore.txn.entitymappers; + +import org.apache.hadoop.hive.metastore.api.MetaException; +import org.apache.hadoop.hive.metastore.txn.CompactionInfo; +import org.apache.hadoop.hive.metastore.txn.MetaWrapperException; +import org.apache.hadoop.hive.metastore.txn.TxnUtils; +import org.springframework.jdbc.core.RowMapper; +import org.springframework.jdbc.core.namedparam.MapSqlParameterSource; +import org.springframework.jdbc.core.namedparam.SqlParameterSource; + +import java.sql.ResultSet; +import java.sql.SQLException; + +public class CompactionInfoMapper implements RowMapper<CompactionInfo>, SqlParamSourceMapper<CompactionInfo> { + + private long compactionEndTime = 0L; + + // language=PostgreSQL + public static final String SELECT = + "SELECT \"CQ_ID\", \"CQ_DATABASE\", \"CQ_TABLE\", \"CQ_PARTITION\", " + + "\"CQ_STATE\", \"CQ_TYPE\", \"CQ_TBLPROPERTIES\", \"CQ_WORKER_ID\", \"CQ_START\", \"CQ_RUN_AS\", " + + "\"CQ_HIGHEST_WRITE_ID\", \"CQ_META_INFO\", \"CQ_HADOOP_JOB_ID\", \"CQ_ERROR_MESSAGE\", " + + "\"CQ_ENQUEUE_TIME\", \"CQ_WORKER_VERSION\", \"CQ_INITIATOR_ID\", \"CQ_INITIATOR_VERSION\", " + + "\"CQ_RETRY_RETENTION\", \"CQ_NEXT_TXN_ID\", \"CQ_TXN_ID\", \"CQ_COMMIT_TIME\", \"CQ_POOL_NAME\", " + + "\"CQ_NUMBER_OF_BUCKETS\", \"CQ_ORDER_BY\" FROM \"COMPACTION_QUEUE\" WHERE \"CQ_ID\" = :id"; + + // language=PostgreSQL + public static final String INSERT = + "INSERT INTO \"COMPLETED_COMPACTIONS\" " + + " (\"CC_ID\", \"CC_DATABASE\", \"CC_TABLE\", \"CC_PARTITION\", \"CC_STATE\", \"CC_TYPE\", " + + " \"CC_TBLPROPERTIES\", \"CC_WORKER_ID\", \"CC_START\", \"CC_END\", \"CC_RUN_AS\", " + + " \"CC_HIGHEST_WRITE_ID\", \"CC_META_INFO\", \"CC_HADOOP_JOB_ID\", \"CC_ERROR_MESSAGE\", " + + " \"CC_ENQUEUE_TIME\", \"CC_WORKER_VERSION\", \"CC_INITIATOR_ID\", \"CC_INITIATOR_VERSION\"," + + " \"CC_NEXT_TXN_ID\", \"CC_TXN_ID\", \"CC_COMMIT_TIME\", \"CC_POOL_NAME\", \"CC_NUMBER_OF_BUCKETS\", " + + " \"CC_ORDER_BY\") " + + " VALUES(:id,:dbname,:tableName,:partName,:state,:type,:properties,:workerId,:start,:endTime," + + " :runAs,:highestWriteId,:metaInfo,:hadoopJobId,:errorMessage,:enqueueTime,:workerVersion,:initiatorId," + + " :initiatorVersion,:nextTxnId,:txnId,:commitTime,:poolName,:numberOfBuckets,:orderByClause)"; + + + public long getCompactionEndTime() { + return compactionEndTime; + } + + public CompactionInfoMapper setCompactionEndTime(long compactionEndTime) { Review Comment: What is the reason of having a setter that returns with the object? ########## standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/entitymappers/SqlParamSourceMapper.java: ########## @@ -0,0 +1,26 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hive.metastore.txn.entitymappers; + +import org.apache.hadoop.hive.metastore.api.MetaException; +import org.springframework.jdbc.core.namedparam.SqlParameterSource; + +public interface SqlParamSourceMapper<T> { Review Comment: What is the intention of that interface? ########## standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/entitymappers/CompactionInfoMapper.java: ########## @@ -0,0 +1,135 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hive.metastore.txn.entitymappers; + +import org.apache.hadoop.hive.metastore.api.MetaException; +import org.apache.hadoop.hive.metastore.txn.CompactionInfo; +import org.apache.hadoop.hive.metastore.txn.MetaWrapperException; +import org.apache.hadoop.hive.metastore.txn.TxnUtils; +import org.springframework.jdbc.core.RowMapper; +import org.springframework.jdbc.core.namedparam.MapSqlParameterSource; +import org.springframework.jdbc.core.namedparam.SqlParameterSource; + +import java.sql.ResultSet; +import java.sql.SQLException; + +public class CompactionInfoMapper implements RowMapper<CompactionInfo>, SqlParamSourceMapper<CompactionInfo> { + + private long compactionEndTime = 0L; + + // language=PostgreSQL + public static final String SELECT = + "SELECT \"CQ_ID\", \"CQ_DATABASE\", \"CQ_TABLE\", \"CQ_PARTITION\", " + + "\"CQ_STATE\", \"CQ_TYPE\", \"CQ_TBLPROPERTIES\", \"CQ_WORKER_ID\", \"CQ_START\", \"CQ_RUN_AS\", " + + "\"CQ_HIGHEST_WRITE_ID\", \"CQ_META_INFO\", \"CQ_HADOOP_JOB_ID\", \"CQ_ERROR_MESSAGE\", " + + "\"CQ_ENQUEUE_TIME\", \"CQ_WORKER_VERSION\", \"CQ_INITIATOR_ID\", \"CQ_INITIATOR_VERSION\", " + + "\"CQ_RETRY_RETENTION\", \"CQ_NEXT_TXN_ID\", \"CQ_TXN_ID\", \"CQ_COMMIT_TIME\", \"CQ_POOL_NAME\", " + + "\"CQ_NUMBER_OF_BUCKETS\", \"CQ_ORDER_BY\" FROM \"COMPACTION_QUEUE\" WHERE \"CQ_ID\" = :id"; + + // language=PostgreSQL + public static final String INSERT = + "INSERT INTO \"COMPLETED_COMPACTIONS\" " + + " (\"CC_ID\", \"CC_DATABASE\", \"CC_TABLE\", \"CC_PARTITION\", \"CC_STATE\", \"CC_TYPE\", " + + " \"CC_TBLPROPERTIES\", \"CC_WORKER_ID\", \"CC_START\", \"CC_END\", \"CC_RUN_AS\", " + + " \"CC_HIGHEST_WRITE_ID\", \"CC_META_INFO\", \"CC_HADOOP_JOB_ID\", \"CC_ERROR_MESSAGE\", " + + " \"CC_ENQUEUE_TIME\", \"CC_WORKER_VERSION\", \"CC_INITIATOR_ID\", \"CC_INITIATOR_VERSION\"," + + " \"CC_NEXT_TXN_ID\", \"CC_TXN_ID\", \"CC_COMMIT_TIME\", \"CC_POOL_NAME\", \"CC_NUMBER_OF_BUCKETS\", " + + " \"CC_ORDER_BY\") " + + " VALUES(:id,:dbname,:tableName,:partName,:state,:type,:properties,:workerId,:start,:endTime," + + " :runAs,:highestWriteId,:metaInfo,:hadoopJobId,:errorMessage,:enqueueTime,:workerVersion,:initiatorId," + + " :initiatorVersion,:nextTxnId,:txnId,:commitTime,:poolName,:numberOfBuckets,:orderByClause)"; + + + public long getCompactionEndTime() { + return compactionEndTime; + } + + public CompactionInfoMapper setCompactionEndTime(long compactionEndTime) { + this.compactionEndTime = compactionEndTime; + return this; + } + + @Override + public CompactionInfo mapRow(ResultSet rs, int rowNum) throws SQLException { + CompactionInfo fullCi = new CompactionInfo(); + fullCi.id = rs.getLong(1); + fullCi.dbname = rs.getString(2); + fullCi.tableName = rs.getString(3); + fullCi.partName = rs.getString(4); + fullCi.state = rs.getString(5).charAt(0);//cq_state Review Comment: Potential null pointer exception. ########## standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CallProperties.java: ########## @@ -0,0 +1,112 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hive.metastore.txn; + +import org.apache.hadoop.hive.metastore.api.MetaException; +import org.datanucleus.transaction.TransactionIsolation; +import org.springframework.transaction.PlatformTransactionManager; + +import java.sql.Connection; +import java.util.function.Function; + +public class CallProperties { Review Comment: What is the intention of that class? ########## standalone-metastore/metastore-server/pom.xml: ########## @@ -23,6 +23,7 @@ <name>Hive Metastore Server</name> <properties> <standalone.metastore.path.to.root>..</standalone.metastore.path.to.root> + <spring.jdbc.version>5.2.24.RELEASE</spring.jdbc.version> Review Comment: Version info should be in root pom -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
