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

feat: Support Case Sensitive Schemas #624

Merged
merged 12 commits into from
Apr 2, 2024
Merged

feat: Support Case Sensitive Schemas #624

merged 12 commits into from
Apr 2, 2024

Conversation

darunrs
Copy link
Collaborator

@darunrs darunrs commented Mar 27, 2024

Indexer schemas can have quoted or unquoted table & column names. However, QueryApi always quotes table names and does not quote column names during SQL query construction for context.db. This is because the AST generated form parsing the schema does not include if the identifier was quoted or not. However, recent updates tot he parsing library has added this functionality in.

I've updated QueryApi to quote both the table name and the column names if they were quoted originally, and leave them unquoted otherwise.

In addition, I've replaced kevin-node-sql-parser back with the original package now that the 5.0 update has released. I've also added a typscript examples folder for convenience, as well as a script to clear the local postgres database.

@darunrs darunrs force-pushed the queryapi-metrics-update branch from 16f7be2 to 2ea5e7a Compare March 27, 2024 22:25
@@ -0,0 +1,30 @@
#!/bin/bash
Copy link
Collaborator Author

@darunrs darunrs Mar 27, 2024

Choose a reason for hiding this comment

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

This is a helpful file for resetting the local PG database if it's ever out of sync with local hasura at any point.

Usage is: Run this script, then run hasura deploy from the hasura folder.

I'll be making progress across PRs to make local testing easier to start up on and work with.

@darunrs darunrs force-pushed the queryapi-metrics-update branch from bf75b0c to bc1f339 Compare March 27, 2024 23:19
@darunrs
Copy link
Collaborator Author

darunrs commented Mar 27, 2024

I tested with the following schema and verified upsert works. Previously, we would get "relation does not exist" errors.

CREATE TABLE
  Posts (
    "id" SERIAL NOT NULL,
    "AccountId" VARCHAR NOT NULL,
    BlockHeight DECIMAL(58, 0) NOT NULL,
    "receiptId" VARCHAR NOT NULL,
    content TEXT NOT NULL,
    block_Timestamp DECIMAL(20, 0) NOT NULL,
    "Accounts_Liked" JSONB NOT NULL DEFAULT '[]',
    "LastCommentTimestamp" DECIMAL(20, 0),
    CONSTRAINT "posts_pkey" PRIMARY KEY ("id")
  );

CREATE TABLE
  "CommentsTable" (
    "id" SERIAL NOT NULL,
    PostId SERIAL NOT NULL,
    "accountId" VARCHAR NOT NULL,
    blockHeight DECIMAL(58, 0) NOT NULL,
    CONSTRAINT "comments_pkey" PRIMARY KEY ("id")
  );

@@ -120,7 +125,7 @@ export default class Indexer {
// TODO: Prevent unnecesary reruns of set status
const resourceCreationSpan = this.tracer.startSpan('prepare vm and context to run indexer code');
simultaneousPromises.push(this.setStatus(functionName, blockHeight, 'RUNNING'));
const vm = new VM({ timeout: 20000, allowAsync: true });
const vm = new VM({ allowAsync: true });
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've removed the timeout as it doesn't even work with async code anyway, right now. I'm also hoping removing the timeout resolves the issues where Pavel's bitmap_v2 indexer is not being traced.

I'll monitor this change and revert it if I need to.

}

private retainOriginalQuoting (schema: string, tableName: string): string {
const createTableQuotedRegex = `\\b(create|CREATE)\\s+(table|TABLE)\\s+"${tableName}"\\s*`;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This regex just looks for a create table statement which uses the quoted tableName. If we find a match, the tableName was quoted.

@@ -0,0 +1,13 @@
// Run with 'npx ts-node src/test-client.ts'
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added typescript examples as well since they're tried to the proto file in the runner folder, and I find messing with them a bit easier.

@darunrs darunrs marked this pull request as ready for review March 28, 2024 00:01
@darunrs darunrs requested a review from a team as a code owner March 28, 2024 00:01
@darunrs darunrs force-pushed the queryapi-metrics-update branch from 0b343a7 to 59e2d1d Compare April 2, 2024 17:48
@darunrs darunrs merged commit ac02424 into main Apr 2, 2024
3 checks passed
@darunrs darunrs deleted the queryapi-metrics-update branch April 2, 2024 17:53
This was referenced Apr 22, 2024
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.

Indexers with quoted case-sensitive column names will have context.db calls fail against them
1 participant