Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(mssql): only last column in foreign key definition kept #1594

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gnarlex
Copy link

@gnarlex gnarlex commented Jul 5, 2024

For SQL Server, when copying foreign key definitions with multiple columns, only the last of N columns is taken over.

The original code tried to aggregate columns in-place, but it seems that some part of the code does not retrieve the original, but rather creates a new list/foreign key object.

Adjust the SQL query, to do the column name aggregation. Then we can avoid having the lisp code to in-place update magic.
(Similar to how it's done in the other DB kind code.)

(I suppose there's great potential to unify the code.)

@RobertoTassistroLeonardo
Copy link

RobertoTassistroLeonardo commented Jul 26, 2024

Very interesting and useful work: in mssql-schema.lisp I had a problem with parentheses... so no compilation
but adding parentheses near the end ";;;" I am able to compile: the program works as expected: really great work! (src/sources/mssql/mssql-schema.lisp)

(defmethod fetch-foreign-keys ((catalog catalog) (mssql copy-mssql)
                               &key including excluding)
  "Get the list of MSSQL foreign key definitions per table."
  (loop
     :with incl-where := (filter-list-to-where-clause
                          mssql including :not nil
                          :schema-col "kcu1.table_schema"
                          :table-col "kcu1.table_name")
     :with excl-where := (filter-list-to-where-clause
                          mssql excluding :not t
                          :schema-col "kcu1.table_schema"
                          :table-col "kcu1.table_name")
     :for (fkey-name schema-name table-name cols
                     fschema-name ftable-name fcols
                     fk-update-rule fk-delete-rule)
     :in (mssql-query (sql "/mssql/list-all-fkeys.sql"
                         (db-name *mssql-db*) (db-name *mssql-db*)
                         incl-where incl-where excl-where excl-where))
     :do (let* ((schema    (find-schema catalog schema-name))
                (table     (find-table schema table-name))
                (fschema   (find-schema catalog fschema-name))
                (ftable    (find-table fschema ftable-name))
                (fkey      (make-fkey :table table
                                      :columns (mapcar #'apply-identifier-case
                                                       (sq:split-sequence #\, cols))
                                      :foreign-table ftable
                                      :foreign-columns (mapcar
                                                        #'apply-identifier-case
                                                        (sq:split-sequence #\, fcols))
                                      :update-rule fk-update-rule
                                      :delete-rule fk-delete-rule)))
;;;   
        (add-fkey table fkey))
;;; 
     :finally (return catalog)))

@dimitri
Copy link
Owner

dimitri commented Jul 30, 2024

It seems like a compilation error was introduced in this PR. Can you see about fixing it before merge?

; compiling file "/home/runner/work/pgloader/pgloader/src/sources/mssql/mssql-schema.lisp" (written 30 JUL 2024 11:25:04 AM):
; processing (IN-PACKAGE :PGLOADER.SOURCE.MSSQL)
; processing (DEFCLASS COPY-MSSQL ...)
; processing (DEFVAR *TABLE-TYPE* ...)
; processing (DEFMETHOD FILTER-LIST-TO-WHERE-CLAUSE ...)
; processing (DEFMETHOD FETCH-COLUMNS ...)
; processing (DEFMETHOD FETCH-INDEXES ...)
; 
; caught ERROR:
;   READ error during COMPILE-FILE:
;   
;     end of file on #<SB-INT:FORM-TRACKING-STREAM for "file /home/runner/work/pgloader/pgloader/src/sources/mssql/mssql-schema.lisp" {100E59DD43}>
;   
;     (in form starting at line: 121, column: 0, position: 5463)

; compilation aborted after 0:00:00.016
While evaluating the form starting at line 21, column 0
  of #P"/home/runner/work/pgloader/pgloader/dumper-2SKVI5f7.lisp":Fatal COMPILE-FILE-ERROR:
  COMPILE-FILE-ERROR while
  compiling #<CL-SOURCE-FILE "pgloader" "src" "sources" "mssql" "mssql-schema">

@gnarlex
Copy link
Author

gnarlex commented Aug 5, 2024

Yes, will fix it later today.
It's due to a misplaced parenthesis.

Fixed it in another PR, but forgot to cherry-pick the fix here.

@RobertoTassistro
Copy link

RobertoTassistro commented Aug 6, 2024

On my system, this Debian Autopkgtest/debian-build works..


name: Debian Autopkgtest

on:
  pull_request: {}
  push: {}

jobs:
  debian-build:
    runs-on: ubuntu-22.04
    steps:
      - name: Checkout code
        uses: actions/checkout@v2

      - name: Set executable permissions
        run: chmod +x debian/tests/testsuite

      - name: Install postgresql-common
        run: sudo apt-get install -y postgresql-common

      - name: Install pgapt repository
        run: sudo /usr/share/postgresql-common/pgdg/apt.postgresql.org.sh -y

      - name: Install dependencies
        run: |
          sudo apt-get update
          sudo apt-get install -y postgresql-server-dev-16
          git clone https://github.com/RhodiumToad/ip4r.git
          cd ip4r
          make
          sudo make install

      - name: Install build-dependencies
        run: sudo apt-get build-dep -y .

      - name: Build pgloader.deb
        run: dpkg-buildpackage --no-sign --buildinfo-option=--version -b

      - name: Install autopkgtest
        run: sudo apt-get install -y autopkgtest

      - name: Autopkgtest
        run: sudo autopkgtest ./ ../pgloader_*_amd64.deb -- null


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.

4 participants