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

pindexer: repair SQL syntax for create type/create domain #4760

Merged
merged 5 commits into from
Jul 25, 2024

Conversation

aubrika
Copy link
Contributor

@aubrika aubrika commented Jul 25, 2024

Fixes some SQL statements in the prior commit which were not valid postgres.

  • If this code contains consensus-breaking changes, I have added the "consensus-breaking" label. Otherwise, I declare my belief that there are not consensus-breaking changes, for the following reason:

    Only indexer changes.

CREATE TYPE Value AS (
amount Amount,
asset BYTEA NOT NULL
asset BYTEA
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the NOT NULL constraint intended here previously is critical, this could be refactored with something like CREATE DOMAIN Asset AS BYTEA NOT NULL; but that might cause translation issues & require a bit more work that this

Not wired up to CI yet, but has proven useful interactively while
debugging #4759.
@conorsch
Copy link
Contributor

This helps, but I'm still seeing errors when I attempt to spin up locally. Running this command should pass, for us to merge this PR:

❯ sqlfluff parse --dialect postgres crates/bin/pindexer/src/dex/dex.sql
[...snip...]
==== parsing violations ====
L:  34 | P:   1 |  PRS | Line 34, Position 1: Found unparsable section: 'CREATE TABLE IF
                       | NOT EXISTS trace_step (\n...'
WARNING: Parsing errors found and dialect is set to 'postgres'. Have you configured your dialect correctly?
❯ echo $?
1

@conorsch
Copy link
Contributor

Much better! The sqlfluff parse <file> command passes now, but I'm still seeing errors on pindexer startup. This time it's:

Error: error returned from database: cannot insert multiple commands into a prepared statement

To test, I'm using the STR in #4759.

@conorsch
Copy link
Contributor

That did it! 💪

@conorsch conorsch self-requested a review July 25, 2024 02:16
conorsch
conorsch previously approved these changes Jul 25, 2024
@conorsch
Copy link
Contributor

Drat, I spoke too soon:

Error: error returned from database: duplicate key value violates unique constraint "block_details_pkey"                                                                                   ```

@conorsch conorsch dismissed their stale review July 25, 2024 02:22

further local testing showed problems

@cronokirby
Copy link
Contributor

That seems to point to a weirder thing in cometindex because that should only happen if you ingest two block root events with the same height

@cronokirby
Copy link
Contributor

#4762 fixes a bug in cometindex which leads to this

@conorsch
Copy link
Contributor

#4762 fixes a bug in cometindex which leads to this

Yup, testing locally with that fix cherry-picked in resolves, thanks. 👍

@conorsch conorsch self-requested a review July 25, 2024 16:22
@conorsch conorsch merged commit 145e959 into main Jul 25, 2024
13 checks passed
@conorsch conorsch deleted the pindexer-sql-fix branch July 25, 2024 16:22
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