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.

Sign up for GitHub

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

fix: type generation on load #648

Merged
merged 6 commits into from
Apr 15, 2024
Merged

fix: type generation on load #648

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
53f66d4
fix: Object traversal for latest node-sql
Kevin101Zhang Apr 8, 2024
0f87935
added additional layer handleCodeGen to onMount
Kevin101Zhang Apr 8, 2024
df3cb90
Remove Whitespace
Kevin101Zhang Apr 8, 2024
a66a944
chore: removed console.log, fixed typo
Kevin101Zhang Apr 9, 2024
088a6b3
Merge branch 'main' of https://github.com/near/queryapi into 629-type…
Kevin101Zhang Apr 10, 2024
3a21e2a
fix: remove redundant call
Kevin101Zhang Apr 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions frontend/src/components/Editor/Editor.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,8 +178,10 @@ const Editor = ({ actionButtonText }) => {
}, [schema, indexingCode]);

useEffect(() => {

localStorage.setItem(SCHEMA_TYPES_STORAGE_KEY, schemaTypes);
attachTypesToMonaco();
handleCodeGen();
Kevin101Zhang marked this conversation as resolved.
Show resolved Hide resolved
}, [schemaTypes, monacoMount]);

useEffect(() => {
Expand All @@ -194,13 +196,12 @@ const Editor = ({ actionButtonText }) => {
}

if (window.monaco) {
// Check if monaco is loaded
// Add generated types to monaco and store disposable to clear them later
const newDisposable =
monaco.languages.typescript.typescriptDefaults.addExtraLib(schemaTypes);
const newDisposable = monaco.languages.typescript.typescriptDefaults.addExtraLib(schemaTypes);
if (newDisposable != null) {
console.log("Types successfully imported to Editor");
}

disposableRef.current = newDisposable;
}
};
Expand Down
45 changes: 26 additions & 19 deletions frontend/src/utils/pgSchemaTypeGen.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ export class PgSchemaTypeGen {
this.tables = new Set();
}

getColumnDefinitionNames (columnDefs) {
getColumnDefinitionNames(columnDefs) {
const columnDefinitionNames = new Map();
for (const columnDef of columnDefs) {
if (columnDef.column?.type === 'column_ref') {
Expand All @@ -18,8 +18,8 @@ export class PgSchemaTypeGen {
}
return columnDefinitionNames;
}
retainOriginalQuoting (schema, tableName) {

retainOriginalQuoting(schema, tableName) {
const createTableQuotedRegex = `\\b(create|CREATE)\\s+(table|TABLE)\\s+"${tableName}"\\s*`;

if (schema.match(new RegExp(createTableQuotedRegex, 'i'))) {
Expand All @@ -28,8 +28,8 @@ export class PgSchemaTypeGen {

return tableName;
}
getTableNameToDefinitionNamesMapping (schema) {

getTableNameToDefinitionNamesMapping(schema) {
let schemaSyntaxTree = this.parser.astify(schema, { database: 'Postgresql' });
schemaSyntaxTree = Array.isArray(schemaSyntaxTree) ? schemaSyntaxTree : [schemaSyntaxTree]; // Ensure iterable
const tableNameToDefinitionNamesMap = new Map();
Expand Down Expand Up @@ -83,32 +83,34 @@ export class PgSchemaTypeGen {
generateTypes(sqlSchema) {
const schemaSyntaxTree = this.parser.astify(sqlSchema, { database: "Postgresql" });
const dbSchema = {};

console.log(schemaSyntaxTree)
Kevin101Zhang marked this conversation as resolved.
Show resolved Hide resolved
const statements = Array.isArray(schemaSyntaxTree) ? schemaSyntaxTree : [schemaSyntaxTree];
// Process each statement in the schema
for (const statement of statements) {
if (statement.type === "create" && statement.keyword === "table") {
// Process CREATE TABLE statements
const tableName = statement.table[0].table;
if (dbSchema.hasOwnProperty(tableName)) {
if (Object.prototype.hasOwnProperty.call(dbSchema, tableName)) {
throw new Error(`Table ${tableName} already exists in schema. Table names must be unique. Quotes are not allowed as a differentiator between table names.`);
}

let columns = {};
for (const columnSpec of statement.create_definitions) {
if (columnSpec.hasOwnProperty("column") && columnSpec.hasOwnProperty("definition")) {

if (Object.prototype.hasOwnProperty.call(columnSpec, "column") && Object.prototype.hasOwnProperty.call(columnSpec, "definition")) {
// New Column
this.addColumn(columnSpec, columns);
} else if (columnSpec.hasOwnProperty("constraint") && columnSpec.constraint_type == "primary key") {
} else if (Object.prototype.hasOwnProperty.call(columnSpec, "constraint") && columnSpec.constraint_type === "primary key") {
// Constraint on existing column
for (const foreignKeyDef of columnSpec.definition) {
columns[foreignKeyDef.column.expr.value].nullable = false;
columns[foreignKeyDef.column].nullable = false;
}
}
}
dbSchema[tableName] = columns;
} else if (statement.type === "alter") {
// Process ALTER TABLE statements
let newConstraint = {};
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's the point in moving the contraint variable outside the for loop? It's scope is to apply to the alterSpec in question.

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 was just due to a stricter eslint configuration where there was a lexical declaration inside a case block. To fix the issue, I moved the lexical declaration outside of the case block.

const tableName = statement.table[0].table;
for (const alterSpec of statement.expr) {
switch (alterSpec.action) {
Expand All @@ -118,7 +120,7 @@ export class PgSchemaTypeGen {
this.addColumn(alterSpec, dbSchema[tableName]);
break;
case "constraint": // Add constraint to column(s) (Only PRIMARY KEY constraint impacts output types)
const newConstraint = alterSpec.create_definitions;
newConstraint = alterSpec.create_definitions;
if (newConstraint.constraint_type == "primary key") {
for (const foreignKeyDef of newConstraint.definition) {
dbSchema[tableName][foreignKeyDef.column].nullable = false;
Expand All @@ -141,11 +143,12 @@ export class PgSchemaTypeGen {
}

addColumn(columnDef, columns) {
const columnName = columnDef.column.column.expr.value;
const columnName = columnDef.column.column;
Copy link
Collaborator

@darunrs darunrs Apr 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Is this correct? I thought the location of the column name was changed after the update? Can you verify this is the right way to get the name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep! This should be it! I verified locally with the latest version we have of node-sql-parser and the object we from astify.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

confirmed by pulling and it works for me

console.log('found the issue ', columnName)
Kevin101Zhang marked this conversation as resolved.
Show resolved Hide resolved
const columnType = this.getTypescriptType(columnDef.definition.dataType);
const nullable = this.getNullableStatus(columnDef);
const required = this.getRequiredStatus(columnDef, nullable);
if (columns.hasOwnProperty(columnName)) {
if (Object.prototype.hasOwnProperty.call(columns, columnName)) {
console.warn(`Column ${columnName} already exists in table. Skipping.`);
return;
}
Expand All @@ -159,17 +162,21 @@ export class PgSchemaTypeGen {

getNullableStatus(columnDef) {
const isPrimaryKey =
columnDef.hasOwnProperty("unique_or_primary") &&
columnDef.unique_or_primary == "primary key";
Object.prototype.hasOwnProperty.call(columnDef, "unique_or_primary") &&
morgsmccauley marked this conversation as resolved.
Show resolved Hide resolved
columnDef.unique_or_primary &&
morgsmccauley marked this conversation as resolved.
Show resolved Hide resolved
columnDef.unique_or_primary === "primary key";
const isNullable =
columnDef.hasOwnProperty("nullable") &&
columnDef.nullable.value == "not null";
Object.prototype.hasOwnProperty.call(columnDef, "nullable")
columnDef.nullable &&
columnDef.nullable.value === "not null";
return isPrimaryKey || isNullable ? false : true;
}

getRequiredStatus(columnDef, nullable) {
const hasDefaultValue =
columnDef.hasOwnProperty("default_val") && columnDef.default_val != null;
Object.prototype.hasOwnProperty.call(columnDef, "default_val") &&
Copy link
Contributor Author

@Kevin101Zhang Kevin101Zhang Apr 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Modified all access methods to avoid shadowing

columnDef.default_val &&
columnDef.default_val != null;
const isSerial = columnDef.definition.dataType
.toLowerCase()
.includes("serial");
Expand Down Expand Up @@ -296,4 +303,4 @@ export class PgSchemaTypeGen {
return "any";
}
}
}
}