fix tests

Author: TrevorBergeron

bigframes/core/sql/__init__.py

@@ -49,8 +49,8 @@
 
 
 def identifier(name: str) -> str:
-    if len(name) > 63:
-        raise ValueError("Identifier must be less than 64 characters")
+    if len(name) > 256:
+        raise ValueError("Identifier must be less than 256 characters")
     return f"`{escape_chars(name)}`"
 
 

bigframes/functions/_function_client.py

@@ -19,6 +19,7 @@
 import logging
 import os
 import random
+import re
 import shutil
 import string
 import tempfile
@@ -196,10 +197,8 @@ def create_bq_remote_function(
         import bigframes.core.utils
 
         # removes anything that isn't letter, number or underscore
-        sql_func_legal_name = bigframes.core.utils.label_to_identifier(
-            name, strict=True
-        )
-        bq_function_name_escaped = bigframes.core.sql.identifier(sql_func_legal_name)
+        _validate_routine_name(name)
+        bq_function_name_escaped = bigframes.core.sql.identifier(name)
         create_function_ddl = f"""
             CREATE OR REPLACE FUNCTION `{self._gcp_project_id}.{self._bq_dataset}`.{bq_function_name_escaped}({udf_def.signature.to_sql_input_signature()})
             RETURNS {udf_def.signature.with_devirtualize().output.sql_type}
@@ -263,7 +262,10 @@ def provision_bq_managed_function(
         # Augment user package requirements with any internal package
         # requirements.
         packages = _utils.get_updated_package_requirements(
-            packages, is_row_processor, capture_references, ignore_package_version=True
+            packages or [],
+            is_row_processor,
+            capture_references,
+            ignore_package_version=True,
         )
         if packages:
             managed_function_options["packages"] = packages
@@ -579,7 +581,7 @@ def provision_bq_remote_function(
         reuse: bool,
         name: str | None,
         package_requirements: tuple[str, ...],
-        max_batching_rows: int,
+        max_batching_rows: int | None,
         cloud_function_timeout: int | None,
         cloud_function_max_instance_count: int | None,
         cloud_function_vpc_connector: str | None,
@@ -591,7 +593,7 @@ def provision_bq_remote_function(
         """Provision a BigQuery remote function."""
         # Augment user package requirements with any internal package
         # requirements
-        package_requirements = _utils.get_updated_package_requirements(
+        full_package_requirements = _utils.get_updated_package_requirements(
             package_requirements, func_signature.is_row_processor
         )
 
@@ -611,7 +613,7 @@ def provision_bq_remote_function(
         concurrency = (workers * threads) if (expected_milli_cpus >= 1000) else 1
 
         cloud_func_spec = udf_def.CloudRunFunctionConfig(
-            code=udf_def.CodeDef.from_func(def_, package_requirements),
+            code=udf_def.CodeDef.from_func(def_, full_package_requirements),
             signature=func_signature,
             timeout_seconds=cloud_function_timeout,
             max_instance_count=cloud_function_max_instance_count,
@@ -655,7 +657,7 @@ def provision_bq_remote_function(
         intended_rf_spec = udf_def.RemoteFunctionConfig(
             endpoint=cf_endpoint,
             connection_id=self._bq_connection_id,
-            max_batching_rows=max_batching_rows,
+            max_batching_rows=max_batching_rows or 1000,
             signature=func_signature,
             bq_metadata=func_signature.protocol_metadata,
         )
@@ -728,6 +730,15 @@ def get_bigframes_function_name(
     return _BQ_FUNCTION_NAME_SEPERATOR.join(parts)
 
 
+def _validate_routine_name(name: str) -> None:
+    """Validate that the given name is a valid BigQuery routine name."""
+    # Routine IDs can contain only letters (a-z, A-Z), numbers (0-9), or underscores (_)
+    if not re.match(r"^[a-zA-Z0-9_]+$", name):
+        raise ValueError(
+            "Routine ID can contain only letters (a-z, A-Z), numbers (0-9), or underscores (_)"
+        )
+
+
 def _infer_milli_cpus_from_memory(memory_mib: int) -> int:
     # observed values, not formally documented by cloud run functions
     if memory_mib < 128:

bigframes/functions/_utils.py

@@ -75,12 +75,12 @@ def _package_existed(package_requirements: list[str], package: str) -> bool:
 
 
 def get_updated_package_requirements(
-    package_requirements=None,
-    is_row_processor=False,
-    capture_references=True,
-    ignore_package_version=False,
-):
-    requirements = []
+    package_requirements: Sequence[str] = (),
+    is_row_processor: bool = False,
+    capture_references: bool = True,
+    ignore_package_version: bool = False,
+) -> Sequence[str]:
+    requirements: list[str] = []
     if capture_references:
         requirements.append(f"cloudpickle=={cloudpickle.__version__}")
 
@@ -110,13 +110,12 @@ def get_updated_package_requirements(
     if not requirements:
         return package_requirements
 
-    if not package_requirements:
-        package_requirements = []
+    result = list(package_requirements)
     for package in requirements:
-        if not _package_existed(package_requirements, package):
-            package_requirements.append(package)
+        if not _package_existed(result, package):
+            result.append(package)
 
-    return sorted(package_requirements)
+    return sorted(result)
 
 
 def clean_up_by_session_id(

bigframes/functions/function.py

@@ -222,7 +222,7 @@ def output_dtype(self):
 
     @property
     def bigframes_bigquery_function_output_dtype(self):
-        return self.output_dtype
+        return self.udf_def.signature.output.emulating_type.bf_type
 
 
 class BigqueryCallableRowRoutine:
@@ -296,4 +296,4 @@ def output_dtype(self):
 
     @property
     def bigframes_bigquery_function_output_dtype(self):
-        return self.output_dtype
+        return self.udf_def.signature.output.emulating_type.bf_type

bigframes/functions/udf_def.py

@@ -295,19 +295,14 @@ def from_routine(
         ## Handle input types
         udf_fields = []
 
-        if is_row_processor:
-            if len(routine.arguments) != 1:
-                raise ValueError(
-                    "Row processor functions must have exactly one input argument."
-                )
-
-        for argument in routine.arguments:
-            if is_row_processor:
-                if argument.data_type.type_kind != "STRING":
+        for i, argument in enumerate(routine.arguments):
+            if is_row_processor and i == 0:
+                if argument.data_type.type_kind == "STRING":
+                    udf_fields.append(UdfArg(argument.name, RowSeriesInputFieldV1()))
+                else:
                     raise ValueError(
-                        "Row processor functions must have STRING input type."
+                        "Row processor functions must have STRING input type as first argument."
                     )
-                udf_fields.append(UdfArg(argument.name, RowSeriesInputFieldV1()))
             udf_fields.append(UdfArg.from_sdk(argument))
 
         return cls(
@@ -317,6 +312,8 @@ def from_routine(
 
     @classmethod
     def from_py_signature(cls, signature: inspect.Signature):
+        import bigframes.series
+
         input_types: list[UdfArg] = []
         for parameter in signature.parameters.values():
             if parameter.annotation is inspect.Signature.empty:
@@ -326,6 +323,10 @@ def from_py_signature(cls, signature: inspect.Signature):
                     f"'{parameter.name}' is missing a type annotation. "
                     "Types are required to use udfs.",
                 )
+            if parameter.annotation is bigframes.series.Series:
+                raise TypeError(
+                    "Argument type hint must be Pandas Series, not BigFrames Series."
+                )
 
             input_types.append(UdfArg.from_py_param(parameter))
 

bigframes/testing/utils.py

@@ -513,7 +513,7 @@ def get_function_name(func, package_requirements=None, is_row_processor=False):
     # Augment user package requirements with any internal package
     # requirements.
     package_requirements = bff_utils.get_updated_package_requirements(
-        package_requirements, is_row_processor
+        package_requirements or [], is_row_processor
     )
 
     # Compute a unique hash representing the user code.

tests/system/large/functions/test_remote_function.py

@@ -33,7 +33,6 @@
 import bigframes.dataframe
 import bigframes.dtypes
 import bigframes.exceptions
-import bigframes.functions._utils as bff_utils
 import bigframes.pandas as bpd
 import bigframes.series
 from bigframes.testing.utils import (
@@ -527,22 +526,6 @@ def add_one(x):
         # Make a unique udf
         add_one_uniq, add_one_uniq_dir = make_uniq_udf(add_one)
 
-        # Expected cloud function name for the unique udf
-        package_requirements = bff_utils.get_updated_package_requirements()
-        add_one_uniq_hash = bff_utils.get_hash(add_one_uniq, package_requirements)
-        add_one_uniq_cf_name = f"bff_{add_one_uniq_hash}_{session.session_id}"
-
-        # There should be no cloud function yet for the unique udf
-        cloud_functions = list(
-            get_cloud_functions(
-                session.cloudfunctionsclient,
-                session.bqclient.project,
-                session.bqclient.location,
-                name=add_one_uniq_cf_name,
-            )
-        )
-        assert len(cloud_functions) == 0
-
         # The first time both the cloud function and the bq remote function don't
         # exist and would be created
         remote_add_one = session.remote_function(
@@ -554,6 +537,9 @@ def add_one(x):
             cloud_function_service_account="default",
         )(add_one_uniq)
 
+        assert remote_add_one.bigframes_cloud_function is not None
+        add_one_uniq_cf_name = remote_add_one.bigframes_cloud_function.split("/")[-1]
+
         # There should have been excactly one cloud function created at this point
         cloud_functions = list(
             get_cloud_functions(
@@ -1561,7 +1547,9 @@ def square(x):
         bq_routine = session.bqclient.get_routine(
             square_remote.bigframes_bigquery_function
         )
-        assert bq_routine.remote_function_options.max_batching_rows == max_batching_rows
+        assert bq_routine.remote_function_options.max_batching_rows == (
+            max_batching_rows or 1000
+        )
 
         scalars_df, scalars_pandas_df = scalars_dfs
 
@@ -1690,7 +1678,8 @@ def test_remote_function_reflects_config_change_with_reuse(session):
         def square(x):
             return x * x
 
-        deploy_name = str(uuid.uuid4())
+        # random alphanumeric name
+        deploy_name = str(uuid.uuid4().hex)
         square_remote = session.remote_function(
             input_types=[int],
             name=deploy_name,

tests/unit/functions/test_remote_function_utils.py

@@ -41,20 +41,6 @@ def test_get_remote_function_locations(
     assert cf_region == expected_cf_region
 
 
-def test_get_updated_package_requirements_no_extra_package():
-    """Tests with no extra package."""
-    result = _utils.get_updated_package_requirements(capture_references=False)
-
-    assert result is None
-
-    initial_packages = ["xgboost"]
-    result = _utils.get_updated_package_requirements(
-        initial_packages, capture_references=False
-    )
-
-    assert result == initial_packages
-
-
 @patch("bigframes.functions._utils.numpy.__version__", "1.24.4")
 @patch("bigframes.functions._utils.pyarrow.__version__", "14.0.1")
 @patch("bigframes.functions._utils.pandas.__version__", "2.0.3")
@@ -100,7 +86,7 @@ def test_get_updated_package_requirements_capture_references_false():
     # Case 1: Only capture_references=False.
     result_1 = _utils.get_updated_package_requirements(capture_references=False)
 
-    assert result_1 is None
+    assert len(result_1) == 0
 
     # Case 2: capture_references=False but is_row_processor=True.
     expected_2 = ["numpy==1.24.4", "pandas==2.0.3", "pyarrow==14.0.1"]