ayushtkn commented on code in PR #6309:
URL: https://github.com/apache/hive/pull/6309#discussion_r2803436317
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java:
##########
@@ -5094,4 +5096,47 @@ public static String getTableOrMVSuffix(Context context,
boolean createTableOrMV
}
return suffix;
}
+
+ /**
+ * Stores the creation time of the given table in the provided configuration.
+ * <p>
+ * The value is written under a composite key of the form:
+ * {@code <dbName>.<tableName>.<CREATE_TIME>}.
+ * </p>
+ *
+ * @param conf
+ * configuration to store the table creation time; must not be {@code
null}
+ * @param table
+ * table whose database and name are used to construct the configuration
key;
+ * must not be {@code null}
+ * @param tableCreateTime
+ * table creation time to store, represented as a string
+ */
+ public static void setTableCreateTime(Configuration conf, Table table) {
+ Objects.requireNonNull(table, "Cannot get table create time. Table object
is expected to be non-null.");
+ String tableCreateTime = String.valueOf(table.getCreateTime());
+ String fullTableName = String.format("%s.%s", table.getDbName(),
table.getTableName());
+ conf.set(String.format("%s.%s", fullTableName, CREATE_TIME),
tableCreateTime);
+ }
+
+ /**
+ * Retrieves the table creation time from the configuration.
+ * <p>
+ * The value is expected to be stored under the key
+ * {@code <tableName>.<CREATE_TIME>}. If the value is not
present,
+ * this method returns {@code 0}.
+ * </p>
+ *
+ * @param conf
+ * configuration containing the table creation time; must not be {@code
null}
+ * @param tableName
+ * fully qualified table name ({@code dbName.tableName})
+ * used to construct the configuration key
+ * @return
+ * the table creation time, or {@code 0} if not set
+ */
+ public static int getTableCreateTime(Configuration conf, String tableName) {
+ String createTime = conf.get(String.format("%s.%s", tableName,
CREATE_TIME));
+ return createTime == null ? 0 : Integer.parseInt(createTime);
Review Comment:
use
```
conf.getInt(...)
```
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java:
##########
@@ -5094,4 +5096,47 @@ public static String getTableOrMVSuffix(Context context,
boolean createTableOrMV
}
return suffix;
}
+
+ /**
+ * Stores the creation time of the given table in the provided configuration.
+ * <p>
+ * The value is written under a composite key of the form:
+ * {@code <dbName>.<tableName>.<CREATE_TIME>}.
+ * </p>
+ *
+ * @param conf
+ * configuration to store the table creation time; must not be {@code
null}
+ * @param table
+ * table whose database and name are used to construct the configuration
key;
+ * must not be {@code null}
+ * @param tableCreateTime
+ * table creation time to store, represented as a string
+ */
+ public static void setTableCreateTime(Configuration conf, Table table) {
+ Objects.requireNonNull(table, "Cannot get table create time. Table object
is expected to be non-null.");
+ String tableCreateTime = String.valueOf(table.getCreateTime());
+ String fullTableName = String.format("%s.%s", table.getDbName(),
table.getTableName());
+ conf.set(String.format("%s.%s", fullTableName, CREATE_TIME),
tableCreateTime);
Review Comment:
We could do setInt, rather than converting it into String and then setting
```
conf.setInt(String.format("%s.%s", fullTableName, CREATE_TIME),
table.getCreateTime());
```
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java:
##########
@@ -5094,4 +5096,47 @@ public static String getTableOrMVSuffix(Context context,
boolean createTableOrMV
}
return suffix;
}
+
+ /**
+ * Stores the creation time of the given table in the provided configuration.
+ * <p>
+ * The value is written under a composite key of the form:
+ * {@code <dbName>.<tableName>.<CREATE_TIME>}.
+ * </p>
+ *
+ * @param conf
+ * configuration to store the table creation time; must not be {@code
null}
+ * @param table
+ * table whose database and name are used to construct the configuration
key;
+ * must not be {@code null}
+ * @param tableCreateTime
+ * table creation time to store, represented as a string
+ */
+ public static void setTableCreateTime(Configuration conf, Table table) {
+ Objects.requireNonNull(table, "Cannot get table create time. Table object
is expected to be non-null.");
+ String tableCreateTime = String.valueOf(table.getCreateTime());
+ String fullTableName = String.format("%s.%s", table.getDbName(),
table.getTableName());
Review Comment:
Can we use:
```
table.getFullyQualifiedName();
```
or
```
TableName.getDbTable(table.getDbName(), table.getTableName());
```
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java:
##########
@@ -5094,4 +5096,47 @@ public static String getTableOrMVSuffix(Context context,
boolean createTableOrMV
}
return suffix;
}
+
+ /**
+ * Stores the creation time of the given table in the provided configuration.
+ * <p>
+ * The value is written under a composite key of the form:
+ * {@code <dbName>.<tableName>.<CREATE_TIME>}.
+ * </p>
+ *
+ * @param conf
+ * configuration to store the table creation time; must not be {@code
null}
+ * @param table
+ * table whose database and name are used to construct the configuration
key;
+ * must not be {@code null}
+ * @param tableCreateTime
+ * table creation time to store, represented as a string
+ */
+ public static void setTableCreateTime(Configuration conf, Table table) {
Review Comment:
this param doesn't exist
##########
ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java:
##########
@@ -3317,13 +3317,23 @@ public static AcidDirectory
getAcidStateFromCache(Supplier<FileSystem> fileSyste
//dbName + tableName + dir
String key = writeIdList.getTableName() + "_" +
candidateDirectory.toString();
DirInfoValue value = dirCache.getIfPresent(key);
+ int tableCreateTimeInCache = value == null ? -1 :
value.getTableCreateTime();
+ int tableCreateTime = Utilities.getTableCreateTime(conf,
writeIdList.getTableName());
// in case of open/aborted txns, recompute dirInfo
long[] exceptions = writeIdList.getInvalidWriteIds();
boolean recompute = (exceptions != null && exceptions.length > 0);
+ // Check whether the table was re-created after being stored in the cache.
+ // The value null check avoids a noisy log message during the initial
lookup, when no cache entry exists.
+ if (value != null && tableCreateTimeInCache < tableCreateTime) {
Review Comment:
The value is in int. Doubting about the Interger Overflow case, Should we
just do
```
tableCreateTimeInCache != tableCreateTime
```
##########
ql/src/test/org/apache/hadoop/hive/ql/plan/TestMapWork.java:
##########
@@ -69,4 +74,35 @@ public void
testDeriveLlapSetsCacheAffinityForTextInputFormat() {
mapWork.getCacheAffinity());
}
+ @Test
+ public void testConfigureJobConfPropagatesTableCreateTime() {
+ // Given a table with a realistic create time
+ String dbName = "test_db";
+ String tableName = "test_table";
+ int createTime = 1770653453;
+
+ Table table = new Table(dbName, tableName);
+ table.setCreateTime(createTime);
+
+ // And a TableScanOperator configured for that table
+ TableScanDesc tsDesc = new TableScanDesc(table);
+ CompilationOpContext cCtx = new CompilationOpContext();
+ TableScanOperator tsOp = new TableScanOperator(cCtx);
+ tsOp.setConf(tsDesc);
+
+ // And a MapWork that uses this TableScanOperator as a root
+ MapWork mapWork = new MapWork();
+ mapWork.getAliasToWork().put("t", tsOp);
+
+ JobConf jobConf = new JobConf();
+
+ // When configuring the job from the MapWork
+ mapWork.configureJobConf(jobConf);
+
+ // Then the table's create time should be present in the JobConf
+ String fullTableName = dbName + "." + tableName;
Review Comment:
```
String fullTableName = TableName.getDbTable(dbName, tableName);
```
##########
ql/src/java/org/apache/hadoop/hive/ql/exec/TableScanOperator.java:
##########
@@ -87,6 +87,15 @@ public class TableScanOperator extends
Operator<TableScanDesc> implements
private ProbeDecodeContext probeDecodeContextSet;
+ @Override
+ public void configureJobConf(JobConf job) {
+ Table table = getConf().getTableMetadata();
Review Comment:
can getConf() be `null`?, maybe we can do, if there is such a chance
```
public void configureJobConf(JobConf job) {
if (getConf() != null && getConf().getTableMetadata() != null) {
Utilities.setTableCreateTime(job, getConf().getTableMetadata());
}
```
--
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]
