Skip to content

Conversation

@gqcn
Copy link
Member

@gqcn gqcn commented Dec 8, 2025

This pull request introduces significant improvements to the handling of the Replace and Save operations for multiple database drivers, especially for MSSQL and PostgreSQL. The changes ensure that these operations now auto-detect primary keys when conflict columns are not explicitly provided, improving usability and aligning behavior across drivers. Additionally, the pull request updates related tests to reflect these enhancements and includes some minor documentation and code cleanup.

Key changes:

Enhanced Replace/Save Logic for Database Drivers

  • MSSQL Driver:

    • Replace and Save operations now auto-detect primary keys if OnConflict is not specified, using the MERGE statement for upsert functionality. If no primary key is found in the data, a detailed error is returned. [1] [2]
    • Updated tests to verify that Replace correctly updates or inserts records, and that missing conflict columns are properly handled. [1] [2]
  • PostgreSQL Driver:

    • Similar to MSSQL, Replace and Save now auto-detect primary keys for conflict resolution if OnConflict is not set, and treat Replace as a Save operation.
    • Adjusted tests to ensure Save and Replace work as expected, including verifying data replacement and insertion. [1] [2] [3]
  • DM Driver:

    • Improved conflict detection: now checks that at least one primary key exists in the provided data when OnConflict is not specified, and provides clearer error messages.
    • Refactored to use the core method for primary key detection and removed redundant code.

Minor Improvements and Documentation

  • Added clarifying comments to DoInsert methods for ClickHouse, DM, MSSQL, Oracle, and PostgreSQL drivers, specifying that the input list must have at least one validated record. [1] [2] [3] [4] [5]
  • Minor code and comment cleanups, including improved formatting and error handling. [1] [2] [3] [4] [5]

Workflow and Documentation Updates

  • Updated example Docker commands in the CI workflow for consistency and clarity. [1] [2] [3] [4] [5] [6]
  • Removed outdated note about Replace support from the SQLite driver documentation.

These changes improve the consistency, reliability, and developer experience when performing upsert operations across different database backends.

gqcn and others added 15 commits December 4, 2025 17:29
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…ey acquisition logic (#4546)

`pgsql driver`中`getPrimaryKeys`未使用现有缓存,导致每次`insert`都会重新查询表字段
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request adds Replace operation support for PostgreSQL and MSSQL drivers by implementing auto-detection of primary keys when conflict columns are not explicitly specified. The changes enhance consistency across database drivers and improve developer experience for upsert operations.

Key changes:

  • Introduced GetPrimaryKeys() method in the core utility layer for retrieving primary keys from table metadata
  • Enhanced MSSQL and PostgreSQL drivers to auto-detect primary keys for Replace and Save operations when OnConflict is not specified
  • Added empty list validation in DriverWrapperDB.DoInsert() to provide clearer error messages
  • Updated tests to verify the new auto-detection behavior and adjusted field type assertions to include precision/length information

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
database/gdb/gdb_core_utility.go Adds GetPrimaryKeys() method to extract primary key field names from table metadata using case-insensitive comparison
database/gdb/gdb_driver_wrapper_db.go Adds validation to return a clear error when data list is empty for insert operations
contrib/drivers/pgsql/pgsql_do_insert.go Implements auto-detection of primary keys for Replace/Save operations, treating Replace as Save with ON CONFLICT
contrib/drivers/pgsql/pgsql_table_fields.go Enhances field type reporting to include precision/length information (e.g., "int4(32)" instead of "int4")
contrib/drivers/pgsql/pgsql_z_unit_*.go Updates tests to verify Replace functionality and adjusts field type assertions to match new format
contrib/drivers/mssql/mssql_do_insert.go Implements auto-detection of primary keys for Replace/Save using MERGE statement, converting Replace to Save operation
contrib/drivers/mssql/mssql_do_exec.go Minor refactoring: renames receiver variable from m to d for consistency
contrib/drivers/mssql/mssql_z_unit_*.go Adds comprehensive tests for Replace operation and updates test expectations for missing primary key scenarios
contrib/drivers/dm/dm_do_insert.go Refactors to use core GetPrimaryKeys() method, removes duplicate code, and improves error messages
contrib/drivers/oracle/oracle_do_insert.go Adds documentation comment and minor formatting cleanup
contrib/drivers/clickhouse/clickhouse_do_insert.go Adds documentation comment about list parameter validation
contrib/drivers/README.MD Removes outdated notes about lack of Replace support for PostgreSQL and MSSQL
.github/workflows/ci-main.yml Updates Docker command examples in comments to remove unnecessary flags for GitHub Actions context

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 40 to 46
foundPrimaryKey := false
for _, conflictKey := range primaryKeys {
if _, ok := list[0][conflictKey]; ok {
foundPrimaryKey = true
break
}
}
Copy link

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potential case sensitivity issue in primary key detection. The code checks list[0][conflictKey] where conflictKey comes from TableFields (e.g., "id" in PostgreSQL). However, if there's any database that returns uppercase field names, user data maps with lowercase keys might not match. Although PostgreSQL returns lowercase field names, this could be an issue for consistency across drivers.

Consider using case-insensitive comparison for consistency:

foundPrimaryKey := false
for _, conflictKey := range primaryKeys {
    for dataKey := range list[0] {
        if strings.EqualFold(dataKey, conflictKey) {
            foundPrimaryKey = true
            break
        }
    }
    if foundPrimaryKey {
        break
    }
}

Copilot uses AI. Check for mistakes.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot open a new pull request to apply changes based on this feedback

LanceAdd and others added 6 commits December 8, 2025 16:48
`gf gen
dao`生成`entity`时支持数据库`uuid`类型字段生成`github.com/google/uuid`的`uuid.UUID`类型字段
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Contributor

Copilot AI commented Dec 8, 2025

@gqcn I've opened a new pull request, #4550, to work on those changes. Once the pull request is ready, I'll request review from you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants