Repository navigation
Conversation
step 1 of quickstart in the repo Signed-off-by: Adrian Edwards <adredwar@redhat.com>
per step 2 of the quickstart Signed-off-by: Adrian Edwards <adredwar@redhat.com>
Signed-off-by: Adrian Edwards <adredwar@redhat.com>
Signed-off-by: Adrian Edwards <adredwar@redhat.com>
Signed-off-by: Adrian Edwards <adredwar@redhat.com>
Signed-off-by: Adrian Edwards <adredwar@redhat.com>
Generated-by: Sonnet 4.6 medium Signed-off-by: Adrian Edwards <adredwar@redhat.com>
Signed-off-by: Adrian Edwards <adredwar@redhat.com>
…l override Signed-off-by: Adrian Edwards <adredwar@redhat.com>
changes made above what sonnet did: - remove a numbered list from the docs and clean up the language a little - change the with_data param to be an override for what we already defined on self, not a straight up setting of the value Generated-by: Sonnet 4.6 Medium Signed-off-by: Adrian Edwards <adredwar@redhat.com>
Signed-off-by: Adrian Edwards <adredwar@redhat.com>
…is silently ignored. Assisted-by: GLM 5.3 Low Signed-off-by: Adrian Edwards <adredwar@redhat.com>
Assisted-by: GLM 5.3 Low Signed-off-by: Adrian Edwards <adredwar@redhat.com>
Generated-by: GLM 5.3 Low Signed-off-by: Adrian Edwards <adredwar@redhat.com>
Generated-by: GLM 5.3 High Signed-off-by: Adrian Edwards <adredwar@redhat.com>
Generated-by: GLM 5.3 High Signed-off-by: Adrian Edwards <adredwar@redhat.com>
Generated-by: GLM 5.3 High Signed-off-by: Adrian Edwards <adredwar@redhat.com>
Generated-by: GLM 5.3 High Signed-off-by: Adrian Edwards <adredwar@redhat.com>
expression-bearing indexes would otherwise render as `op.create_index(..., [])` (broken SQL) Generated-by: GLM 5.3 High Signed-off-by: Adrian Edwards <adredwar@redhat.com>
Unsure about whether the remaining ones should be added individually in multiple indexes or together in one compound index Generated-by: GLM 5.3 Low Signed-off-by: Adrian Edwards <adredwar@redhat.com>
collectoss-new-schema.sql does these as compound indexes. This is likely quite inefficient and could use review Generated-by: GLM 5.3 Low Signed-off-by: Adrian Edwards <adredwar@redhat.com>
Signed-off-by: Adrian Edwards <adredwar@redhat.com>
Signed-off-by: Adrian Edwards <adredwar@redhat.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| db = db_sigs.get(name) | ||
| if db is None: | ||
| # Added (or renamed-to) index. | ||
| logger.info( |
There was a problem hiding this comment.
[pylint] reported by reviewdog 🐶
E1205: Too many arguments for logging format string (logging-too-many-args)
| # list): CREATE INDEX cannot overwrite, so drop + create. | ||
| # (If either side is expression-bearing — columns is None | ||
| # — the comparison above is name-only; skip.) | ||
| logger.info( |
There was a problem hiding this comment.
[pylint] reported by reviewdog 🐶
E1205: Too many arguments for logging format string (logging-too-many-args)
| # Removed (or renamed-from) index. Columns are attached | ||
| # to the reverse op only when parseable, so the | ||
| # downgrade recreates it faithfully. | ||
| logger.info( |
There was a problem hiding this comment.
[pylint] reported by reviewdog 🐶
E1205: Too many arguments for logging format string (logging-too-many-args)
| # comparator. MUST be imported after register_entities above: the entity | ||
| # comparator registers first, so views with a pending create/replace op | ||
| # (which already render index creation) are skipped by the index comparator. | ||
| from collectoss.application.db import matview_index_comparator # noqa: E402,F401 |
There was a problem hiding this comment.
[pylint] reported by reviewdog 🐶
W0611: Unused matview_index_comparator imported from collectoss.application.db (unused-import)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This change is essentially a v2 of #292. It aims to address problems discovered in #449.
The previous system of matview versioning accepted a tuple of strings (
unique_index_columns) that were turned into anCREATE INDEXSQL call. It was intended only for the columns that have unique indexes (i.e. a candidate key that allows for concurrent matview refresh to happen).Some of the views coming in via #449 do not have candidate keys, or had their candidate keys removed in 8Knot. This (i believe since its been a while) is the main driving force behind needing a more robust materialized view versioning system that can support a wider range of index types.
Notes for Reviewers
this PR introduces yet another library -
alembic_utils_extended(AUE), a fork ofalembic_utilsthat can handle passing regular SqlalchemyIndexobjects to materialized views.It has a key weakness, which is that it only drops/recreates the matview when its definition changes. if only an index changes, then this is not detected.
When asking about this upstream, it seems as though the answer is to write a custom comparator, which this change also includes.
Testing
While this change includes unit tests for the new comparator, this still needs extensive testing to ensure it is reliable, since this is core data schema versioning functionality.
needed:
Other Todos:
Signed commits
Generative AI disclosure
Please select one option:
If AI tools were used, please provide details below:
- What tools were used? Sonnet 4.6 medium via cursor, GLM 5.3 Low/High via Claude Code CLI
- How were these tools used? Code edits and drafting of changes, writing unit tests
- Did you review these outputs before submitting this PR? The non-test code has been reviewed and manually edited as it was committed and also afterwards via additional commits.