-
Notifications
You must be signed in to change notification settings - Fork 0
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
SQL examples draft #158
SQL examples draft #158
Conversation
(cherry picked from commit b7b87bc)
src/main/metabase/examples.ts
Outdated
function rawGroupBySQL(column: string, table: string, displayName: string): ExampleInfo { | ||
return { | ||
name: `${displayName} by ${column}`, | ||
sql: lines(`SELECT ${postgresQuote(column)}`, `FROM ${postgresQuote(table)}`), |
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.
Is GROUP BY clause missing?
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.
yup, lost in refactors
src/main/metabase/examples.ts
Outdated
function countDistinctSQL(column: string, table: string): ExampleInfo { | ||
return { | ||
name: `Distinct ${column}`, | ||
sql: lines(`SELECT count(distinct ${postgresQuote(column)}) as distinct_${column}`, `FROM ${postgresQuote(table)}`), |
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.
Do we handle quoting of this? If so, we might get distinct_"some col"
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.
I think I checked mb reports the table unquoted here, so we should be fine. I'll check again
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.
unquoted, but I have to prevent distinct_some col
, I'll fix this
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.
I'll fix this
Let's fix bugs in a future PR. I want to synchronize and experiment often.
try { | ||
if (field.semantic_type === 'type/PK' || field.database_type === 'serial') { | ||
// No sensible example for columns being just row IDs. | ||
return []; | ||
} else if (aidColumns.includes(field.name)) { |
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.
I'd split (in a future revision) all these into their own function, i.e. aidColumnExamples
, numericColumnExamples
, textColumnExamples
, etc.
Also a future idea is to get some columns with very few distinct values and try grouping by 2 columns. If the values are distributed enough we might get non-suppressed results.
638ab98
to
1c57920
Compare
Part of #77. I didn't test these, nor I don't know how do they present.
Edon: Feel free to just plug this into your scaffolding, and once this is somewhat working, I can pick up again and start polishing the looks and rules.