The Wayback Machine - https://web.archive.org/web/20201205144953/https://github.com/dimitri/pgloader/pull/970
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

Generic Function API for Materialized Views support. #970

Merged
merged 6 commits into from May 20, 2019

Conversation

@dimitri
Copy link
Owner

@dimitri dimitri commented May 18, 2019

Generalize support code for Materialized Views support, as it is about the same for the three databases where we have support for it.

dimitri added 2 commits May 6, 2019
Implement a generic-function API to discover the source database schema and
populate pgloader internal version of the catalogs.
Cut down three copies of about the same code-path down to a single shared
one, thanks to applying some amount of OOP to the code.
@dimitri dimitri changed the title Gf api for schema listing Generic Function API for Materialized Views support. May 18, 2019
@dimitri dimitri self-assigned this May 18, 2019
(:documentation "pgloader connection parameters for DBF files."))
(defclass copy-db3 (db-copy)
((encoding :accessor encoding ; file encoding
:initarg :encoding))

This comment has been minimized.

@phoe

phoe May 19, 2019
Collaborator

This line mixes tabs and spaces.

This comment has been minimized.

@dimitri

dimitri May 19, 2019
Author Owner

It's my Emacs setup that does that, I think. And I think it's just standard SLIME setup here. In other words I don't mind too much about that.

src/sources/mssql/mssql-connection.lisp Outdated Show resolved Hide resolved
src/sources/mssql/mssql-schema.lisp Outdated Show resolved Hide resolved
src/sources/mssql/mssql-schema.lisp Outdated Show resolved Hide resolved
src/sources/mssql/mssql-schema.lisp Outdated Show resolved Hide resolved
src/sources/sqlite/sqlite-schema.lisp Outdated Show resolved Hide resolved
src/sources/sqlite/sqlite-schema.lisp Outdated Show resolved Hide resolved
src/sources/mssql/mssql-schema.lisp Outdated Show resolved Hide resolved
src/sources/mssql/mssql-schema.lisp Outdated Show resolved Hide resolved
src/sources/mssql/mssql-schema.lisp Outdated Show resolved Hide resolved
  - Some initialize-instance :after left over code can be removed.

    The casting nowadays happens within the catalogs rather than the COPY
    objects, and the copy objects are derived from the preparation work we
    do on the catalogs. The casting machinery is called in process-catalog
    in src/load/migrate-database.lisp

  - It's more lisp-idiomatic to benefit from loop features when using loop
    anyway, so rather than using let to bind values inside the body of the
    loop we can with :with var := instead.

  - Fix some copy/paste'os in MS SQL where we still referenced MySQL.
@phoe

This comment has been minimized.

Copy link
Collaborator

@phoe phoe commented on src/sources/mssql/mssql-schema.lisp in f881a6b May 19, 2019

AFAIK these LET* bindings can be further transformed into FOR bindings in LOOP to avoid a LET* inside LOOP.

@phoe

This comment has been minimized.

Copy link
Collaborator

@phoe phoe commented on src/sources/mssql/mssql-schema.lisp in f881a6b May 19, 2019

Ditto.

@phoe

This comment has been minimized.

Copy link
Collaborator

@phoe phoe commented on src/sources/mssql/mssql-schema.lisp in f881a6b May 19, 2019

Now that I look at it, however, I kind of wonder if keeping the LET* in here wouldn't be somewhat rational. Here, the LET* is a clear boundary between what we iterate over, and what we do for each iteration step.

dimitri added 2 commits May 19, 2019
Move the (format nil "sql" arg1 arg2) into the SQL function, which makes
more sense on an abstraction layer pespective and makes the calling code
easier to read.
@phoe

This comment has been minimized.

Copy link
Collaborator

@phoe phoe commented on src/utils/queries.lisp in e9eaa10 May 19, 2019

This may signal a type error. (gethash url *fs*) may return NIL which is not a valid format string. The OR clause is therefore badly constructed and the ERROR call in line 65 is unreachable.

This comment has been minimized.

Copy link
Owner Author

@dimitri dimitri replied May 19, 2019

Fixed.

@dimitri
Copy link
Owner Author

@dimitri dimitri commented May 19, 2019

This may signal a type error. (gethash url *fs*) may return NIL which is not a valid format string. The OR clause is therefore badly constructed and the ERROR call in line 65 is unreachable.

Thanks, good spot! Fixed now!

@dimitri dimitri merged commit b8da7dd into master May 20, 2019
4 checks passed
4 checks passed
ci/dockercloud Your tests passed in Docker Cloud
Details
ci/dockercloud (/Dockerfile.ccl) Your tests passed in Docker Cloud
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@dimitri dimitri deleted the gf-api-for-schema-listing branch May 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
You can’t perform that action at this time.