tvalentyn commented on code in PR #40225:
URL: https://github.com/apache/beam/pull/40225#discussion_r4169574856


##########
sdks/python/apache_beam/io/gcp/bigquery.py:
##########
@@ -789,6 +792,10 @@ def estimate_size(self):
         table_ref.projectId = self._get_project()
       table = bq.get_table(
           table_ref.projectId, table_ref.datasetId, table_ref.tableId)
+      if table.numBytes is None:
+        # Tables that don't report storage statistics, e.g. Lakehouse runtime
+        # catalog (BigLake metastore) tables.
+        return 0

Review Comment:
   should this return None? Not sure what's the diff between 0 and None, I 
haven't look closely, but above here we also return None in this method.



##########
sdks/python/apache_beam/io/gcp/bigquery.py:
##########
@@ -1256,14 +1265,18 @@ def split(self, desired_bundle_size, 
start_position=None, stop_position=None):
         requested_session.read_options.row_restriction = self.row_restriction
 
       storage_client = bq_storage.BigQueryReadClient()
+      # A stream_count of 0 lets the Storage Read API choose the number of
+      # streams; used when the table reports no size (e.g. Lakehouse runtime
+      # catalog tables).
       stream_count = 0
-      if desired_bundle_size > 0:
-        table_size = self._get_table_size(bq, self.table_reference)
-        stream_count = min(
-            int(table_size / desired_bundle_size),
-            _CustomBigQueryStorageSource.MAX_SPLIT_COUNT)
-      stream_count = max(
-          stream_count, _CustomBigQueryStorageSource.MIN_SPLIT_COUNT)
+      table_size = self._get_table_size(bq, self.table_reference)
+      if table_size is not None:

Review Comment:
   From readability, this might be easier to follow
   
   ```
         if table_size is None:
           # comment
           stream_count = 0
         else:
           stream_count = 0
           ...
   ```
   
   or perphaps `get_stream_count` could be a helper function   



##########
sdks/python/apache_beam/io/gcp/bigquery.py:
##########
@@ -29,7 +29,10 @@
 Also, for programming convenience, instances of TableReference and TableSchema
 have a string representation that can be used for the corresponding arguments:
 
-  - TableReference can be a PROJECT:DATASET.TABLE or DATASET.TABLE string.
+  - TableReference can be a PROJECT:DATASET.TABLE, PROJECT.DATASET.TABLE or
+    DATASET.TABLE string. Lakehouse runtime catalog (BigLake metastore) tables

Review Comment:
   Since this is new functionality, can we force a standard format without 
dot/colon ambiguity and without potentially ambiguous parsing 
(project.dataset.table vs [no_project].catalog.namespace.table)? For example we 
could change the docstring to:
   
   TableReference can be a PROJECT:DATASET.TABLE, PROJECT.DATASET.TABLE, 
DATASET.TABLE, or PROJECT.CATALOG.NAMESPACE.TABLE (for Lakehouse catalog 
tables). 
   
   



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