Conversation
…lity This change updates the UdfParameter.parse method to use the database dialect for determining the correct quote character (backticks for GoogleSQL, double quotes for PostgreSQL). It also refactors the string processing logic to avoid manual index manipulation, leveraging regex-based splitting for the DEFAULT expression. Additionally, it includes a fix for a compilation error in DirectRunnerClient.java by replacing a deprecated/missing stop() call with interrupt(). New unit tests have been added to verify correct parsing of quoted identifiers and default expressions across dialects.
This change adds integration tests to ExportPipelineIT and ImportPipelineIT to verify that User-Defined Functions (UDFs) are correctly exported and imported across different Spanner dialects (GoogleSQL and PostgreSQL). - Added UDF definitions to test DDL files. - Updated ExportPipelineIT to verify UDF artifacts in GCS. - Added testPostgresImportPipeline_UDF to ImportPipelineIT to verify UDF restoration from Avro artifacts. - Included necessary Avro and manifest resources for the import test.
- Replaced manual index manipulation with regex in UdfParameter.parse for better readability and robust dialect support. - Added support for PostgreSQL types with spaces (e.g., 'double precision') in UdfParameter.parse. - Preserved GoogleSQL single-word type restriction to minimize behavioral changes. - Added unit tests for PostgreSQL types with spaces. - Fixed unused import in InformationSchemaScannerTest.java.
- Delete pr_comments.json per PR review feedback. - Fix Ddl.Builder.createUdf to pass dialect to Udf.builder(dialect) so scanned PostgreSQL UDFs retain Dialect.POSTGRESQL. - Fix PostgreSQL UDF parameter quoting and determinism assertions in ExportPipelineIT. - Add missing pg_multiply.avro resource and fix manifest filenames and Base64 MD5 checksums for ImportPipelineIT. - Reintroduce PostgreSQL UDF coverage in CopyDbIT-AllSchemaAndData-pg.sql and AvroSchemaToDdlConverterTest. TAG=agy CONV=f1c413a3-3f8a-43cd-8eac-9b7a2df210e0
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces support for User-Defined Functions (UDFs) within the PostgreSQL dialect for Spanner Dataflow templates. It enables the export and import of PostgreSQL UDFs by mapping determinism levels to volatility keywords and enhancing parameter parsing to handle quoted identifiers and default values. These changes ensure consistency and compatibility when migrating or exporting Spanner databases using the PostgreSQL dialect. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds support for User-Defined Functions (UDFs) in the PostgreSQL dialect for Cloud Spanner, including metadata extraction, Avro schema conversion, and SQL generation. The review feedback highlights critical improvements to the parsing logic in UdfParameter.parse, such as fixing a regex bug where Pattern.quote is incorrectly used inside a character class, and addressing a split-logic issue that could bypass empty default expression checks. Additionally, suggestions are made to increase robustness by trimming input parameters and handling case-insensitivity for determinism values. All comments are highly actionable and should be addressed.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4240 +/- ##
============================================
+ Coverage 55.85% 56.33% +0.48%
+ Complexity 7488 7412 -76
============================================
Files 1135 1155 +20
Lines 70313 73245 +2932
Branches 8040 8589 +549
============================================
+ Hits 39271 41263 +1992
- Misses 28490 29192 +702
- Partials 2552 2790 +238
🚀 New features to boost your workflow:
|
…ameter - Trim parameter string at the start of UdfParameter.parse. - Pass quote character directly in UdfParameter.parse regex. - Use word boundary (?i)\s+DEFAULT\b when splitting DEFAULT expression in UdfParameter.parse. - Match spannerDeterminism() case-insensitively in Udf.prettyPrint. - Add unit tests in UdfParameterTest and UdfTest. TAG=agy CONV=f1c413a3-3f8a-43cd-8eac-9b7a2df210e0
…rameter scan - Filter out EXTERNAL routines (such as PostgreSQL change stream read_json_* functions) in InformationSchemaScanner.listFunctionParametersSQL() to match listUdfs() and listUdfOptions(). - Move UDFs in ExportPipelineIT, ImportPipelineIT, and CopyDbIT into named schemas, as Cloud Spanner disallows UDFs in the default schema. - Replace unsupported 'AS ...' SQL function bodies in PostgreSQL DDL test resources with 'RETURN (SELECT ...)'. - Update PostgreSQL UDF type/definition/default expectations in ExportPipelineIT, ImportPipelineIT, and InformationSchemaScannerIT to match Spanner PostgreSQL information_schema output. TAG=agy CONV=f1c413a3-3f8a-43cd-8eac-9b7a2df210e0
2 participants
Adding support for UDFs in the PostgreSQL dialect.
Note, this PR is based on the previously reviewed #3325.
This PR has been rebased, review comments addressed, and a few test failures addressed.