tvalentyn commented on code in PR #40225:
URL: https://github.com/apache/beam/pull/40225#discussion_r4170237341
##########
sdks/python/apache_beam/io/gcp/bigquery.py:
##########
@@ -29,10 +29,11 @@
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, PROJECT.DATASET.TABLE or
- DATASET.TABLE string. Lakehouse runtime catalog (BigLake metastore) tables
- use PROJECT.CATALOG.NAMESPACE.TABLE, which maps to a composite
- CATALOG.NAMESPACE dataset id.
+ - TableReference can be a PROJECT:DATASET.TABLE, PROJECT.DATASET.TABLE,
+ DATASET.TABLE or, for Lakehouse runtime catalog (BigLake metastore)
+ tables, PROJECT.CATALOG.NAMESPACE.TABLE, which maps to a composite
+ CATALOG.NAMESPACE dataset id. A Lakehouse reference must include the
+ project id: a three-part string is read as PROJECT.DATASET.TABLE.
Review Comment:
nit: I'd drop : 'a three-part string is read as PROJECT.DATASET.TABLE',
since it's slightly confusing as statement after : might be interpreted as an
illustration of the statement before :, which is not the case (it is and
explanation).
##########
sdks/python/apache_beam/io/gcp/bigquery.py:
##########
@@ -1122,6 +1123,22 @@ def _get_table_size(self, bq, table_reference):
# runtime catalog (BigLake metastore) tables.
return table.numBytes
+ def _get_stream_count(self, bq, desired_bundle_size):
+ """Number of streams to request, or 0 to let the Storage Read API decide.
+
+ A table that reports no size, e.g. a Lakehouse runtime catalog (BigLake
Review Comment:
I'd move:
A table that reports no size, e.g. a Lakehouse runtime catalog (BigLake
metastore) table, cannot be split by size.
in-line where we return 0 at line 1134.
##########
sdks/python/apache_beam/io/gcp/bigquery.py:
##########
@@ -29,10 +29,11 @@
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, PROJECT.DATASET.TABLE or
- DATASET.TABLE string. Lakehouse runtime catalog (BigLake metastore) tables
- use PROJECT.CATALOG.NAMESPACE.TABLE, which maps to a composite
- CATALOG.NAMESPACE dataset id.
+ - TableReference can be a PROJECT:DATASET.TABLE, PROJECT.DATASET.TABLE,
+ DATASET.TABLE or, for Lakehouse runtime catalog (BigLake metastore)
Review Comment:
is biglake an outdated / no longer used product name? if so, you can omit it
for better readability.
##########
sdks/python/apache_beam/io/gcp/bigquery_tools.py:
##########
@@ -288,24 +298,83 @@ def parse_table_reference(table, dataset=None,
project=None):
# table argument will contain a full table reference instead of just a
# table name.
if dataset is None:
- pattern = (
- f'((?P<project>{_PROJECT_PATTERN})[:\\.])?'
- f'(?P<dataset>{_DATASET_PATTERN})\\.(?P<table>{_TABLE_PATTERN})')
- match = regex.fullmatch(pattern, table)
- if not match:
- raise ValueError(
- 'Expected a table reference (PROJECT:DATASET.TABLE or '
- 'DATASET.TABLE) instead of %s.' % table)
- table_reference.projectId = match.group('project')
- table_reference.datasetId = match.group('dataset')
- table_reference.tableId = match.group('table')
+ project_id, dataset_id, table_id = _split_table_spec(table)
+ table_reference.projectId = project_id
+ table_reference.datasetId = dataset_id
+ table_reference.tableId = table_id
else:
table_reference.projectId = project
table_reference.datasetId = dataset
table_reference.tableId = table
return table_reference
+def _invalid_table_spec(table_spec):
+ return ValueError(
+ 'Expected a table reference (PROJECT:DATASET.TABLE, '
+ 'PROJECT.DATASET.TABLE, DATASET.TABLE, '
+ 'PROJECT:CATALOG.NAMESPACE.TABLE or PROJECT.CATALOG.NAMESPACE.TABLE) '
+ 'instead of %s.' % table_spec)
+
+
+def _split_table_spec(table_spec):
+ """Splits a table spec string into (project, dataset, table).
+
+ The regex only validates the character set; segment assignment is done by
+ explicit splitting so that composite Lakehouse dataset ids ('catalog.ns')
+ and domain-scoped project ids ('example.com:proj') are both handled. This
+ mirrors BigQueryHelpers.parseTableSpec in the Java SDK.
+ """
+ pattern = (
+ f'((?P<project>{_PROJECT_PATTERN})[:\\.])?'
+ f'(?P<dataset>{_DATASET_PATTERN})\\.(?P<table>{_TABLE_PATTERN})')
+ if not regex.fullmatch(pattern, table_spec):
+ raise _invalid_table_spec(table_spec)
+
+ # Table ids cannot contain '.', so the table is always the last segment.
+ last_dot = table_spec.rfind('.')
+ table = table_spec[last_dot + 1:]
+ prefix = table_spec[:last_dot]
+
+ colon_count = prefix.count(':')
+ if colon_count == 0:
+ # Purely dotted form: 'p.d.t', 'd.t', 'p.catalog.ns.t'. The leading
+ # segment is the project when it looks like one; dataset ids may contain
+ # characters such as '_' that project ids may not, in which case the
Review Comment:
re: > in which case the whole prefix is the dataset
I think we should disallow this pattern in code and tests given the updated
intent:
```
> A Lakehouse reference must include the project id
```
--
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]
