Skip to content

Conversation

mjsir911
Copy link

@mjsir911 mjsir911 commented Sep 7, 2025

Fixes #25146

This is a breaking change since it messes with the AST a little bit to go from:

nkForeignKey
  nkIdent artist_id_artist
  nkReferences
    nkCall
      nkIdent artist
      nkIdent artist_id

to

nkForeignKey
  nkColumnList
    nkIdent artist_id_artist
  nkReferences
    nkIdent artist
    nkColumnList
      nkIdent artist_id

Which I honestly feel fine/good about, especially considering it lines up closer to https://www.sqlite.org/syntax/table-constraint.html + https://www.sqlite.org/syntax/foreign-key-clause.html & represents the constraint columnes & referenced columns as potential lists, as opposed to calls as it was before (why was it parsing this as a call? I see why but also it makes no sense)

@mjsir911 mjsir911 force-pushed the m/parsesql-render-foreign-fix branch 2 times, most recently from 79cd09a to 7b49951 Compare September 7, 2025 09:39
@mjsir911 mjsir911 changed the title WIP: Foreign keys clauses don't have parens or comms rework sqlparse foreignkey ast & renderer a bit Sep 7, 2025
@mjsir911 mjsir911 force-pushed the m/parsesql-render-foreign-fix branch from 7b49951 to bdc199b Compare September 7, 2025 09:41
Fixes nim-lang#25146

This is a breaking change since it messes with the AST a little bit to
go from:

    nkForeignKey
      nkIdent artist_id_artist
      nkReferences
        nkCall
          nkIdent artist
          nkIdent artist_id

to

    nkForeignKey
      nkColumnList
        nkIdent artist_id_artist
      nkReferences
        nkIdent artist
        nkColumnList
          nkIdent artist_id

Which I honestly feel fine/good about, especially considering it lines
up closer to https://www.sqlite.org/syntax/table-constraint.html +
https://www.sqlite.org/syntax/foreign-key-clause.html & represents the
constraint columnes & referenced columns as potential lists, as opposed
to calls as it was before
(why was it parsing this as a call? I see why but also it makes no
sense)
@Araq
Copy link
Member

Araq commented Sep 9, 2025

Ormin also had to fork parsesql, time to move it into a Nimble package of its own. There you can apply this patch then.

Ping @ringabout

@ringabout ringabout self-assigned this Sep 9, 2025
@ringabout
Copy link
Member

progress: #25156

@Araq
Copy link
Member

Araq commented Sep 11, 2025

Apply it at our new Nimble package please.

@Araq Araq closed this Sep 11, 2025
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.

parsesql re-rendering foreign constrains incorrectly
3 participants