luoyuxia commented on a change in pull request #16430:
URL: https://github.com/apache/flink/pull/16430#discussion_r687316685



##########
File path: 
flink-connectors/flink-connector-hive/src/main/java/org/apache/flink/connectors/hive/HiveTableSource.java
##########
@@ -133,66 +123,22 @@ public boolean isBounded() {
 
     @VisibleForTesting
     protected DataStream<RowData> getDataStream(StreamExecutionEnvironment 
execEnv) {
-        validateScanConfigurations();
-        checkAcidTable(catalogTable, tablePath);
+        HiveSourceBuilder sourceBuilder =
+                new HiveSourceBuilder(jobConf, flinkConf, tablePath, 
hiveVersion, catalogTable);
         List<HiveTablePartition> allHivePartitions =
                 getAllPartitions(
                         jobConf,
                         hiveVersion,
                         tablePath,
-                        catalogTable,
-                        hiveShim,
+                        catalogTable.getPartitionKeys(),
                         remainingPartitions);
-        Configuration configuration = 
Configuration.fromMap(catalogTable.getOptions());
 
-        HiveSource.HiveSourceBuilder sourceBuilder =
-                new HiveSource.HiveSourceBuilder(
-                        jobConf,
-                        tablePath,
-                        catalogTable,
-                        allHivePartitions,
-                        limit,
-                        hiveVersion,
-                        
flinkConf.get(HiveOptions.TABLE_EXEC_HIVE_FALLBACK_MAPRED_READER),
-                        (RowType) getProducedDataType().getLogicalType());
-        if (isStreamingSource()) {
-            if (catalogTable.getPartitionKeys().isEmpty()) {
-                PartitionOrder partitionOrder = 
configuration.get(STREAMING_SOURCE_PARTITION_ORDER);
-                if (partitionOrder != PartitionOrder.CREATE_TIME) {
-                    throw new UnsupportedOperationException(
-                            "Only '"
-                                    + PartitionOrder.CREATE_TIME
-                                    + "' is supported for non partition 
table.");
-                }
-            }
-
-            Duration monitorInterval =
-                    configuration.get(STREAMING_SOURCE_MONITOR_INTERVAL) == 
null
-                            ? DEFAULT_SCAN_MONITOR_INTERVAL
-                            : 
configuration.get(STREAMING_SOURCE_MONITOR_INTERVAL);
-            sourceBuilder.monitorContinuously(monitorInterval);
-
-            if (!catalogTable.getPartitionKeys().isEmpty()) {
-                sourceBuilder.setFetcher(new HiveContinuousPartitionFetcher());
-                final String defaultPartitionName =
-                        jobConf.get(
-                                HiveConf.ConfVars.DEFAULTPARTITIONNAME.varname,
-                                
HiveConf.ConfVars.DEFAULTPARTITIONNAME.defaultStrVal);
-                HiveContinuousPartitionFetcherContext<?> fetcherContext =
-                        new HiveContinuousPartitionFetcherContext(
-                                tablePath,
-                                hiveShim,
-                                new JobConfWrapper(jobConf),
-                                catalogTable.getPartitionKeys(),
-                                getTableSchema().getFieldDataTypes(),
-                                getTableSchema().getFieldNames(),
-                                configuration,
-                                defaultPartitionName);
-                sourceBuilder.setFetcherContext(fetcherContext);
-            }
-        }
+        sourceBuilder
+                .setPartitions(allHivePartitions)

Review comment:
       Is it needed to set [artitions in here?  In streaming mode, I think it's 
no need to set parition for it will try to fetch paritions instead of use the  
partitions set; And in batch mode, if not set partitions, by default, 
HiveSourceBuilder will get all paritions and set the all partitions.

##########
File path: 
flink-connectors/flink-connector-hive/src/main/java/org/apache/flink/connectors/hive/HiveTableSource.java
##########
@@ -133,66 +123,22 @@ public boolean isBounded() {
 
     @VisibleForTesting
     protected DataStream<RowData> getDataStream(StreamExecutionEnvironment 
execEnv) {
-        validateScanConfigurations();
-        checkAcidTable(catalogTable, tablePath);
+        HiveSourceBuilder sourceBuilder =
+                new HiveSourceBuilder(jobConf, flinkConf, tablePath, 
hiveVersion, catalogTable);
         List<HiveTablePartition> allHivePartitions =
                 getAllPartitions(
                         jobConf,
                         hiveVersion,
                         tablePath,
-                        catalogTable,
-                        hiveShim,
+                        catalogTable.getPartitionKeys(),
                         remainingPartitions);
-        Configuration configuration = 
Configuration.fromMap(catalogTable.getOptions());
 
-        HiveSource.HiveSourceBuilder sourceBuilder =
-                new HiveSource.HiveSourceBuilder(
-                        jobConf,
-                        tablePath,
-                        catalogTable,
-                        allHivePartitions,
-                        limit,
-                        hiveVersion,
-                        
flinkConf.get(HiveOptions.TABLE_EXEC_HIVE_FALLBACK_MAPRED_READER),
-                        (RowType) getProducedDataType().getLogicalType());
-        if (isStreamingSource()) {
-            if (catalogTable.getPartitionKeys().isEmpty()) {
-                PartitionOrder partitionOrder = 
configuration.get(STREAMING_SOURCE_PARTITION_ORDER);
-                if (partitionOrder != PartitionOrder.CREATE_TIME) {
-                    throw new UnsupportedOperationException(
-                            "Only '"
-                                    + PartitionOrder.CREATE_TIME
-                                    + "' is supported for non partition 
table.");
-                }
-            }
-
-            Duration monitorInterval =
-                    configuration.get(STREAMING_SOURCE_MONITOR_INTERVAL) == 
null
-                            ? DEFAULT_SCAN_MONITOR_INTERVAL
-                            : 
configuration.get(STREAMING_SOURCE_MONITOR_INTERVAL);
-            sourceBuilder.monitorContinuously(monitorInterval);
-
-            if (!catalogTable.getPartitionKeys().isEmpty()) {
-                sourceBuilder.setFetcher(new HiveContinuousPartitionFetcher());
-                final String defaultPartitionName =
-                        jobConf.get(
-                                HiveConf.ConfVars.DEFAULTPARTITIONNAME.varname,
-                                
HiveConf.ConfVars.DEFAULTPARTITIONNAME.defaultStrVal);
-                HiveContinuousPartitionFetcherContext<?> fetcherContext =
-                        new HiveContinuousPartitionFetcherContext(
-                                tablePath,
-                                hiveShim,
-                                new JobConfWrapper(jobConf),
-                                catalogTable.getPartitionKeys(),
-                                getTableSchema().getFieldDataTypes(),
-                                getTableSchema().getFieldNames(),
-                                configuration,
-                                defaultPartitionName);
-                sourceBuilder.setFetcherContext(fetcherContext);
-            }
-        }
+        sourceBuilder
+                .setPartitions(allHivePartitions)

Review comment:
       Is it needed to set partitions in here?  In streaming mode, I think it's 
no need to set parition for it will try to fetch paritions instead of use the  
partitions set; And in batch mode, if not set partitions, by default, 
HiveSourceBuilder will get all paritions and set the all partitions.




-- 
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]


Reply via email to