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

Option for .as() output to be wrapped in double-quotes? #576

Open
echentw opened this issue Apr 29, 2023 · 2 comments
Open

Option for .as() output to be wrapped in double-quotes? #576

echentw opened this issue Apr 29, 2023 · 2 comments
Labels
enhancement New feature or request

Comments

@echentw
Copy link
Contributor

echentw commented Apr 29, 2023

A use case is if we want to give an expression an alias that includes special characters, e.g.

db.columnName.as('?column?')

Right now, the generated SQL is column_name ?column?, which is invalid SQL. Desired output: column_name "?column?".

Maybe related: #243

If this seems reasonable, I'm happy to work on a PR.
Maybe the usage could look like:

db.columnName.as('?column?', {quoted: true})
@echentw echentw added the enhancement New feature or request label Apr 29, 2023
@cakoose
Copy link
Collaborator

cakoose commented May 1, 2023

Looks like Mammoth tries to automatically wrap in quotes, but its heuristic is off:

export const wrapQuotes = (string: string, extended?: boolean) => {
const isCamelCase = string.match(/[A-Z]/);
const isReserved = reservedKeywords.has(string) || (extended && allReservedKeywords.has(string));
const containsSpace = string.includes(` `);
const shouldWrap = isReserved || isCamelCase || containsSpace;
return shouldWrap ? `"${string}"` : string;
};

From the Postgres manual (link):

SQL identifiers and key words must begin with a letter (a-z, but also letters with diacritical marks and non-Latin letters) or an underscore (_). Subsequent characters in an identifier or key word can be letters, underscores, digits (0-9), or dollar signs ($). Note that dollar signs are not allowed in identifiers according to the letter of the SQL standard, so their use might render applications less portable. The SQL standard will not define a key word that contains digits or starts or ends with an underscore, so identifiers of this form are safe against possible conflict with future extensions of the standard.

So maybe we should replace the isCamelCase check with:

// More restrictive than strictly necessary, so we'll end up quoting some things that don't necessarily need quotes.
const isValidWithoutQuotes = /^[a-z_][A-Za-z0-9_$]*$/;

1 similar comment
@cakoose
Copy link
Collaborator

cakoose commented May 1, 2023

Looks like Mammoth tries to automatically wrap in quotes, but its heuristic is off:

export const wrapQuotes = (string: string, extended?: boolean) => {
const isCamelCase = string.match(/[A-Z]/);
const isReserved = reservedKeywords.has(string) || (extended && allReservedKeywords.has(string));
const containsSpace = string.includes(` `);
const shouldWrap = isReserved || isCamelCase || containsSpace;
return shouldWrap ? `"${string}"` : string;
};

From the Postgres manual (link):

SQL identifiers and key words must begin with a letter (a-z, but also letters with diacritical marks and non-Latin letters) or an underscore (_). Subsequent characters in an identifier or key word can be letters, underscores, digits (0-9), or dollar signs ($). Note that dollar signs are not allowed in identifiers according to the letter of the SQL standard, so their use might render applications less portable. The SQL standard will not define a key word that contains digits or starts or ends with an underscore, so identifiers of this form are safe against possible conflict with future extensions of the standard.

So maybe we should replace the isCamelCase check with:

// More restrictive than strictly necessary, so we'll end up quoting some things that don't necessarily need quotes.
const isValidWithoutQuotes = /^[a-z_][A-Za-z0-9_$]*$/;

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

No branches or pull requests

2 participants