Skip to content

Commit

Permalink
omit schema
Browse files Browse the repository at this point in the history
  • Loading branch information
mmkal committed May 7, 2024
1 parent 3c6867b commit 329f6fd
Show file tree
Hide file tree
Showing 7 changed files with 87 additions and 50 deletions.
Expand Up @@ -30,8 +30,13 @@ export function useTrpcClient() {
},
}),
queryCache: new QueryCache({
onError: error => {
toast.warning(String(error))
onError: (error, query) => {
toast.error(
<div>
{String(error)}
<pre>Query: {query.queryHash}</pre>
</div>,
)
},
}),
}),
Expand Down
2 changes: 1 addition & 1 deletion packages/admin/src/components/ui/sonner.tsx
Expand Up @@ -17,7 +17,7 @@ const Toaster = ({...props}: ToasterProps) => {
toastOptions={{
classNames: {
toast:
'group toast DISABLEDgroup-[.toaster]:bg-background group-[.toaster]:text-foreground group-[.toaster]:border-border group-[.toaster]:shadow-lg',
'group toast bg-white DISABLEDgroup-[.toaster]:bg-background group-[.toaster]:text-foreground group-[.toaster]:border-border group-[.toaster]:shadow-lg',
error: 'bg-red-500 text-white',
description: 'group-[.toast]:text-muted-foreground',
actionButton: 'group-[.toast]:bg-primary group-[.toast]:text-primary-foreground',
Expand Down
3 changes: 3 additions & 0 deletions packages/admin/src/server/middleware.ts
Expand Up @@ -10,6 +10,9 @@ const createClientMemoized = pMemoize(async (connectionString: string) => {

export const apiMiddleware = createExpressMiddleware({
router: appRouter,
onError: props => {
console.error('trpc error', props.error)
},
createContext: async ({req}) => {
const connectionString =
req.headers['connection-string']?.toString() ||
Expand Down
28 changes: 26 additions & 2 deletions packages/migrator/src/migrator.ts
Expand Up @@ -372,6 +372,31 @@ export class Migrator extends umzug.Umzug<MigratorContext> {
await writeFile(filepath, migration.sql)
}

/**
* Get a new instance of `this`. Options passed will be spread with `migratorOptions` passed to the constructor of the current instance.
* In subclasses with different constructor parameters, this should be overridden to return an instance of the subclass.
*
* @example
* ```ts
* class MyMigrator extends Migrator {
* options: MyMigratorOptions
* constructor(options: MyMigratorOptions) {
* super(convertMyOptionsToBaseOptions(options))
* this.options = options
* }
*
* cloneWith(options?: MigratorOptions) {
* const MigratorClass = this.constructor as typeof MyMigrator
* const myOptions = convertBaseOptionsToMyOptions(options)
* return new MyMigrator({...this.options, ...options})
* }
* ```
*/
cloneWith(options?: Partial<MigratorOptions>) {
const MigratorClass = this.constructor as typeof Migrator
return new MigratorClass({...this.migratorOptions, ...options})
}

/**
* @experimental
* Creates a "down" migration equivalent to the specified "up" migration.
Expand All @@ -384,8 +409,7 @@ export class Migrator extends umzug.Umzug<MigratorContext> {
pgpOptions: this.client.pgpOptions,
})

const MigratorClass = this.constructor as typeof Migrator // todo: how do we know we can pass the same kind of constructor parameters to this
const migrator = new MigratorClass({...this.migratorOptions, client})
const migrator = this.cloneWith({client})

const create = () => this.client.query(sql`create database ${sql.identifier([dbName])}`)

Expand Down
14 changes: 5 additions & 9 deletions packages/schemainspect/src/inspected.ts
@@ -1,7 +1,7 @@
/* eslint-disable @typescript-eslint/no-explicit-any */
/* eslint-disable @typescript-eslint/no-unsafe-argument */
import {AutoThisAssigner, SomeOptional, SomeRequired} from './auto-this'
import {AutoRepr, quoted_identifier, unquoted_identifier} from './misc'
import {AutoRepr, SchemaIdentifier, quoted_identifier, quotify, unquoted_identifier} from './misc'
import {InspectedConstraint, InspectedEnum, InspectedIndex} from './pg'
import {Relationtype} from './types'

Expand Down Expand Up @@ -35,11 +35,11 @@ export abstract class Inspected extends AutoRepr {
}

get quoted_name() {
return quoted_identifier(this.name)
return quotify(this.name)
}

get quoted_schema() {
return quoted_identifier(this.schema)
return quotify(this.schema)
}

// todo: __ne__
Expand All @@ -63,10 +63,6 @@ export interface TableRelated {
quoted_full_table_name: string
}

export const getQuotedFullTableName = (thing: {schema: string; table_name: string}) => {
return `${quoted_identifier(thing.schema)}.${quoted_identifier(thing.table_name)}`
}

interface ColumnInfoOptions {
name: string
dbtype?: string
Expand Down Expand Up @@ -173,7 +169,7 @@ export class ColumnInfo extends AutoThisAssigner<ColumnInfoOptions, typeof AutoR
}

get quoted_name(): string {
return quoted_identifier(this.name)
return quotify(this.name)
}

get creation_clause(): string {
Expand Down Expand Up @@ -244,7 +240,7 @@ export class ColumnInfo extends AutoThisAssigner<ColumnInfoOptions, typeof AutoR
}

get collation_subclause(): string {
const collate = this.collation ? ` collate ${quoted_identifier(this.collation)}` : ''
const collate = this.collation ? ` collate ${quotify(this.collation)}` : ''
return collate
}

Expand Down
34 changes: 23 additions & 11 deletions packages/schemainspect/src/misc.ts
Expand Up @@ -53,14 +53,25 @@ export const unquoted_identifier = (identifier: string, schema?: string, identit

export const canSkipQuotes = (identifier: string) => /^[_a-z]+$/.test(identifier)

export const quoted_identifier = (identifier: string, schema?: string, identity_arguments?: string) => {
// if (canSkipQuotes(identifier) && !schema && !identity_arguments) {
// return identifier // no need for quotes
// }

// if (canSkipQuotes(identifier) && canSkipQuotes(schema) && !identity_arguments) {
// return `${schema}.${identifier}` // no need for quotes
// }
// deviation: the original `schemainstpect` project has a single `quoted_identifier` function that receives schema and identifier.
// this separates out the quoting from the schema part, because some migrations assume that the schema is inferred from the search path
// (e.g. applications that run migrations multiple times on different schemas - it's important *not* to include the hard-coded schema in those cases)
// so, `quoted_identifier` now *must* specify its schema. This avoids usage like `${quoted_identifier('myschema')}.${quoted_identifier('mytable')}`
// which is used in some places in the original project. Instead, it's now `${quoted_identifier('mytable', 'myschema')}` so that the schema is always explicit.
// that way, we can check against a configured search path to omit the schema.
export const quotify = (identifier: string) => identifier.replaceAll(`"`, `""`)

export const quoted_identifier = (
identifier: string,
schema: string,
// identity_arguments?: string, // deviation: removed to avoid overloads (see above). only used two places, they just have to do it manually
): string => {
// todo: use async storage for search path, not an env var.
const searchPathParts = process.env.SCHEMAINSPECT_SEARCH_PATH?.split(',') || []
if (searchPathParts.includes(schema)) {
// console.log('shortcut', {schema, searchPathParts})
return quotify(identifier)
}

if (!identifier && schema) {
return `"${schema.replaceAll(`"`, '""')}"`
Expand All @@ -71,9 +82,10 @@ export const quoted_identifier = (identifier: string, schema?: string, identity_
s = `"${schema.replaceAll(`"`, `""`)}".${s}`
}

if (identity_arguments) {
s += `(${identity_arguments})`
}
// deviation: removed, see above
// if (identity_arguments) {
// s += `(${identity_arguments})`
// }

return s
}
Expand Down
47 changes: 22 additions & 25 deletions packages/schemainspect/src/pg/obj.ts
Expand Up @@ -17,10 +17,10 @@ import {Queryable, createClient, sql} from '@pgkit/client'
import * as path from 'path'
import {AutoThisAssigner} from '../auto-this'
import {TopologicalSorter} from '../graphlib'
import {ColumnInfo, Inspected, BaseInspectedSelectable, TableRelated, getQuotedFullTableName, pick} from '../inspected'
import {ColumnInfo, Inspected, BaseInspectedSelectable, TableRelated, pick} from '../inspected'
import {DBInspector} from '../inspector'
import {asa, isa} from '../isa-asa'
import {quoted_identifier, getResourceText} from '../misc'
import {quoted_identifier, getResourceText, quotify} from '../misc'
import {Queries} from '../types'
import {groupBy, isEqual} from '../util'

Expand Down Expand Up @@ -416,15 +416,15 @@ export class InspectedTrigger extends AutoThisAssigner<InspectedTriggerParams, t
}

get quoted_full_name(): string {
return `${quoted_identifier(this.schema)}.${quoted_identifier(this.table_name)}.${quoted_identifier(this.name)}`
return `${quoted_identifier(this.table_name, this.schema)}.${quotify(this.name)}`
}

get quoted_full_selectable_name(): string {
return `${quoted_identifier(this.schema)}.${quoted_identifier(this.table_name)}`
return quoted_identifier(this.table_name, this.schema)
}

get drop_statement(): string {
return `drop trigger if exists "${this.name}" on "${this.schema}"."${this.table_name}";`
return `drop trigger if exists "${this.name}" on ${quoted_identifier(this.table_name, this.schema)};`
}

get create_statement(): string {
Expand All @@ -434,11 +434,11 @@ export class InspectedTrigger extends AutoThisAssigner<InspectedTriggerParams, t
R: 'ENABLE REPLICA TRIGGER',
A: 'ENABLE ALWAYS TRIGGER',
}
const schema = quoted_identifier(this.schema)
const table = quoted_identifier(this.table_name)
const trigger_name = quoted_identifier(this.name)
// const schema = quoted_identifier(this.schema)
// const table = quoted_identifier(this.table_name)
const trigger_name = quotify(this.name)
if (['D', 'R', 'A'].includes(this.enabled)) {
const table_alter = `ALTER TABLE ${schema}.${table} ${status_sql[this.enabled]} ${trigger_name}`
const table_alter = `ALTER TABLE ${this.quoted_full_selectable_name} ${status_sql[this.enabled]} ${trigger_name}`
return `${this.full_definition};\n${table_alter};`
}

Expand Down Expand Up @@ -485,7 +485,7 @@ export interface InspectedIndexParams {
export const InspectedIndexParent = AutoThisAssigner<InspectedIndexParams, typeof Inspected>(Inspected)
export class InspectedIndex extends InspectedIndexParent implements TableRelated {
get quoted_full_table_name() {
return getQuotedFullTableName(this)
return quoted_identifier(this.table_name, this.schema)
}

get drop_statement(): string {
Expand Down Expand Up @@ -581,7 +581,7 @@ export class InspectedSequence extends AutoThisAssigner<InspectedSequenceParams,

get quoted_table_and_column_name(): string | undefined {
if (this.column_name && this.table_name) {
return `${this.quoted_full_table_name}.${quoted_identifier(this.column_name)}`
return `${this.quoted_full_table_name}.${quotify(this.column_name)}`
}

return undefined
Expand Down Expand Up @@ -650,7 +650,7 @@ export class InspectedEnum extends AutoThisAssigner<InspectedEnumParams, typeof

alter_rename_statement(new_name: string): string {
const name = new_name
return `alter type ${this.quoted_full_name} rename to ${quoted_identifier(name)};`
return `alter type ${this.quoted_full_name} rename to ${quotify(name)};`
}

drop_statement_with_rename(new_name: string): string {
Expand Down Expand Up @@ -725,7 +725,7 @@ export class InspectedSchema extends Inspected {
}

get quoted_name(): string {
return quoted_identifier(this.schema)
return quotify(this.schema)
}

toJSON(): Record<string, unknown> {
Expand All @@ -744,7 +744,7 @@ export class InspectedType extends AutoThisAssigner<InspectedTypeParams, typeof
let sql = `create type ${this.signature} as (\n`

const indent = ' '.repeat(4)
const typespec = Object.entries(this.columns).map(([name, _type]) => `${indent}${quoted_identifier(name)} ${_type}`)
const typespec = Object.entries(this.columns).map(([name, _type]) => `${indent}${quotify(name)} ${_type}`)

sql += typespec.join(',\n')
sql += '\n);'
Expand Down Expand Up @@ -862,7 +862,7 @@ export class InspectedConstraint extends InspectedConstraintParent implements Ta
fk_columns_foreign: string | null

get quoted_full_table_name() {
return getQuotedFullTableName(this)
return quoted_identifier(this.table_name, this.schema)
}

constructor(params: InspectedConstraintParams) {
Expand Down Expand Up @@ -928,7 +928,7 @@ export class InspectedConstraint extends InspectedConstraintParent implements Ta
}

get quoted_full_name(): string {
return `${quoted_identifier(this.schema)}.${quoted_identifier(this.table_name)}.${quoted_identifier(this.name)}`
return `${quoted_identifier(this.table_name, this.schema)}.${quotify(this.name)}`
}

toJSON(): Record<string, unknown> {
Expand Down Expand Up @@ -961,7 +961,7 @@ export class InspectedPrivilege extends InspectedPrivilegeParent {
}

get quoted_target_user(): string {
return quoted_identifier(this.target_user)
return quotify(this.target_user)
}

get drop_statement(): string {
Expand Down Expand Up @@ -1029,7 +1029,7 @@ export class InspectedRowPolicy extends InspectedRowPolicyParent implements Tabl
}

get quoted_full_table_name(): string {
return getQuotedFullTableName(this)
return quoted_identifier(this.table_name, this.schema)
}

get key(): string {
Expand Down Expand Up @@ -1319,12 +1319,9 @@ export class PostgreSQL extends DBInspector {
this.deps = [...q]

for (const dep of this.deps) {
const x = quoted_identifier(dep.name, dep.schema, dep.identity_arguments)
const x_dependent_on = quoted_identifier(
dep.name_dependent_on,
dep.schema_dependent_on,
dep.identity_arguments_dependent_on,
)
const parenthesize = (expr: string) => (expr ? `(${expr})` : '')
const x = `${quoted_identifier(dep.name, dep.schema)}${parenthesize(dep.identity_arguments)}`
const x_dependent_on = `${quoted_identifier(dep.name_dependent_on, dep.schema_dependent_on)}${parenthesize(dep.identity_arguments_dependent_on)}`
this.selectables[x].dependent_on.push(x_dependent_on)
isa.array(this.selectables[x].dependent_on, String)
this.selectables[x].dependent_on.sort()
Expand Down Expand Up @@ -1517,7 +1514,7 @@ export class PostgreSQL extends DBInspector {
return null
}

const quoted_full_name = `${quoted_identifier(schema)}.${quoted_identifier(name)}`
const quoted_full_name = `${quoted_identifier(name, schema)}`

return this.enums[quoted_full_name]
}
Expand Down

0 comments on commit 329f6fd

Please sign in to comment.