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: Custom schema for SQL query visualization #51

Merged
merged 35 commits into from
Nov 8, 2023
Merged

Conversation

sanjibansg
Copy link
Contributor

This PR adds support for defining custom schema in the SQL CodeView. Previously, querying only on the lineitem table was allowed through SQL in the fiddle, but with this feature, different table schemas can be defined and queries can be visualized.

@sanjibansg sanjibansg marked this pull request as ready for review October 11, 2023 19:25
api/app.py Outdated

def verify_token(token: str = Depends(oauth2_scheme)):
try:
payload = jwt.decode(token, "key", algorithms=["HS256"])
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this shall be replaced by a ENV variable

const payload = {
user_id: user_id,
};
const secret = new TextEncoder().encode("key");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

will be present in an ENV variable

api/app.py Outdated Show resolved Hide resolved
api/app.py Outdated Show resolved Hide resolved
api/app.py Outdated Show resolved Hide resolved
api/app.py Outdated Show resolved Hide resolved
api/backend/duckdb.py Outdated Show resolved Hide resolved
client/src/App.vue Outdated Show resolved Hide resolved
client/src/assets/js/substrait-parser.js Show resolved Hide resolved
api/backend/duckdb.py Outdated Show resolved Hide resolved
conn.load_extension("substrait")
self.conn_pool.append(conn)

def check_pool(self):
Copy link

Choose a reason for hiding this comment

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

why is this function here if we're doing something similar in the init?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's mostly to check if the pool has active connection instances before providing one in the get_connection() method below. I have moved the code to a different method to avoid code repetitions.

@richtia
Copy link

richtia commented Oct 11, 2023

Can we add docstrings to some of python functions to get a better understanding of what they're doing?

@sanjibansg
Copy link
Contributor Author

Can we add docstrings to some of python functions to get a better understanding of what they're doing?

Yes, considering the big change, docstrings will be good for giving a better understanding of the workflow. I will add them.

api/test.py Outdated Show resolved Hide resolved
api/app.py Outdated Show resolved Hide resolved
@sanjibansg sanjibansg requested a review from richtia November 2, 2023 16:03

# Pool size is default at 5 for maintaining
# 5 concurrent DuckDB connection objects
POOL_SIZE = 5
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we should also make this as an ENV variable

@sanjibansg sanjibansg added the enhancement New feature or request label Nov 2, 2023
@sanjibansg
Copy link
Contributor Author

The Cypress Test in electron is failing for an unrelated issue. I will investigate and make a different PR for that.

@sanjibansg sanjibansg merged commit bfdda28 into main Nov 8, 2023
7 of 8 checks passed
@sanjibansg sanjibansg deleted the custom-schema branch November 26, 2023 04:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DuckDB connection as a state management instance Support for custom schema for SQL queries
3 participants