-
Notifications
You must be signed in to change notification settings - Fork 38.8k
fix(core): Fix some DataStore backend API issues #17978
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
base: ADO-3851
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cubic analysis
2 issues found across 4 files • Review in cubic
React with 👍 or 👎 to teach cubic. You can also tag @cubic-dev-ai
to give feedback, ask questions, or re-run the review.
default: | ||
throw new NotFoundError(`Unsupported field type: ${type as string}`); | ||
} | ||
} | ||
|
||
function columnToWildcardAndType(column: DataStoreCreateColumnSchema) { | ||
return `\`${column.name}\` ${dataStoreColumnTypeToSql(column.type)}`; | ||
return `\`${column.name}\` ${dataStoreColumnTypeToSql(column.type)}`; // Postgres identifiers use double quotes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Column names are quoted with back-ticks even when targeting Postgres, which requires double quotes, leading to invalid SQL at runtime (Based on your team's feedback about avoiding backend SQL mismatches).
Prompt for AI agents
Address the following comment on packages/cli/src/modules/data-store/utils/sql-utils.ts at line 29:
<comment>Column names are quoted with back-ticks even when targeting Postgres, which requires double quotes, leading to invalid SQL at runtime (Based on your team's feedback about avoiding backend SQL mismatches).</comment>
<file context>
@@ -19,14 +19,14 @@ function dataStoreColumnTypeToSql(type: DataStoreCreateColumnSchema['type']) {
case 'boolean':
return 'BOOLEAN';
case 'date':
- return 'DATETIME';
+ return 'DATETIME'; // Postgres has no DATETIME
default:
throw new NotFoundError(`Unsupported field type: ${type as string}`);
}
}
</file context>
@@ -19,14 +19,14 @@ | |||
case 'boolean': | |||
return 'BOOLEAN'; | |||
case 'date': | |||
return 'DATETIME'; | |||
return 'DATETIME'; // Postgres has no DATETIME |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Returns DATETIME
even though the comment notes the type is not supported by Postgres, so queries generated for Postgres will fail (Based on your team's feedback about catching backend SQL mismatches).
Prompt for AI agents
Address the following comment on packages/cli/src/modules/data-store/utils/sql-utils.ts at line 22:
<comment>Returns `DATETIME` even though the comment notes the type is not supported by Postgres, so queries generated for Postgres will fail (Based on your team's feedback about catching backend SQL mismatches).</comment>
<file context>
@@ -19,14 +19,14 @@ function dataStoreColumnTypeToSql(type: DataStoreCreateColumnSchema['type']) {
case 'boolean':
return 'BOOLEAN';
case 'date':
- return 'DATETIME';
+ return 'DATETIME'; // Postgres has no DATETIME
default:
throw new NotFoundError(`Unsupported field type: ${type as string}`);
</file context>
Fixes to unblock Adore. Tests now recognize entities, but they now fail because
sql-utils.ts
needs to be made sensitive to DB type.