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

WIP: Reduce the number of SQL queries #342

Closed
wants to merge 84 commits into from
Closed
Show file tree
Hide file tree
Changes from 80 commits
Commits
Show all changes
84 commits
Select commit Hold shift + click to select a range
8dc0b9b
fix(postgres): fix processing money type
benjie Feb 1, 2017
c390862
Move the common select fragment into its own file
benjie Feb 2, 2017
cd22083
Some type browsing within resolveInfo
benjie Feb 2, 2017
b614cf3
Args
benjie Feb 2, 2017
6d94bf7
Nearly there
benjie Feb 2, 2017
3a85c3b
Make fields a map from name to SQL fragment
benjie Feb 2, 2017
e4f3939
Actually query
benjie Feb 2, 2017
c266056
Send resolveInfo through to read
benjie Feb 2, 2017
4b1208c
Support non-relay format
benjie Feb 2, 2017
b9ebaea
Desperate tweaks
benjie Feb 2, 2017
0ba7e1f
POPME
benjie Feb 2, 2017
8435b27
I can't believe it's working
benjie Feb 2, 2017
59e05b3
This commit doesn't exist, you're imagining things
benjie Feb 2, 2017
9738fcf
Use SQL name for fields rather than GQL name
benjie Feb 2, 2017
e25b80d
More tweaking
benjie Feb 2, 2017
16a5129
Typo
benjie Feb 2, 2017
bc8ec2b
Neater implementation
benjie Feb 3, 2017
e6668f3
Working using type._typeConfig hack
benjie Feb 3, 2017
48865dd
Alias procedure columns so can call multiple times with different arg…
benjie Feb 3, 2017
b4c3606
Use graphql-js internal argument getter
benjie Feb 3, 2017
8822038
Unused
benjie Feb 3, 2017
a9d0b81
Rename function
benjie Feb 3, 2017
6cf1af6
Reformat
benjie Feb 3, 2017
f6bf5f0
Add support for InlineFragments
benjie Feb 3, 2017
6e7819f
Safer against future refactoring
benjie Feb 3, 2017
a2c8c97
Pass field name through to sqlName/sqlExpression
benjie Feb 4, 2017
12f9428
Convert paginator to use only SQL
benjie Feb 4, 2017
47d8f9b
More familiar
benjie Feb 4, 2017
e3ebd86
Beginnings of SQL-ification of PGPaginatorOrderingAttributes
benjie Feb 4, 2017
5bb3898
Move hasNextPage/hasPreviousPage into SQL
benjie Feb 4, 2017
9a5c720
Prepare to allow connections via subqueries
benjie Feb 4, 2017
948200f
Simple fake resolveInfo
benjie Feb 4, 2017
fbcee96
sqlField rather than sqlExpression
benjie Feb 4, 2017
7a14a42
Almost working, except you can't use with in subqueries
benjie Feb 4, 2017
c0ce393
NOBODY MOVE! It seems to be working...
benjie Feb 4, 2017
8ad4521
Support for PgPaginatorOrderingOffset
benjie Feb 4, 2017
051ac2d
Rename sql* to external*
benjie Feb 6, 2017
94061d7
Syntax fixes
benjie Feb 6, 2017
af230a0
Revert back to old access method
benjie Feb 6, 2017
69e142d
Why?
benjie Feb 6, 2017
bfda962
Safer DataLoader
benjie Feb 6, 2017
94d0898
More familiar PgProcedure API
benjie Feb 6, 2017
34fcd85
Add totalCount into the optimisation mix
benjie Feb 6, 2017
5444de6
Tidy map logic
benjie Feb 6, 2017
a3dc29a
Remove unnecessary change
benjie Feb 6, 2017
2b4195e
Fix alias lookup
benjie Feb 6, 2017
54091ee
No need for async
benjie Feb 6, 2017
8d08073
Add valueToPage to interface
benjie Feb 6, 2017
3808e37
Safeties
benjie Feb 6, 2017
a9b5f45
row_number() counts from 1
benjie Feb 6, 2017
5b01610
f
benjie Feb 6, 2017
4276920
Make external dependencies explicit
benjie Feb 6, 2017
3ca379c
Make JSON field names into literals
benjie Feb 6, 2017
bedc861
Add dependencies for relations
benjie Feb 6, 2017
3df8863
Stash fields for later execution to allow merging
benjie Feb 6, 2017
9a4ec44
Use subquery setting in config
benjie Feb 6, 2017
2e747e6
Add missing collection type
benjie Feb 6, 2017
878368b
Default to subquery mode
benjie Feb 6, 2017
997a165
All procedures can be executed in the same way
benjie Feb 6, 2017
a8bae58
Procedure support
benjie Feb 6, 2017
9d869fe
Casting to prevent SQL confusion
benjie Feb 6, 2017
814008e
Handle null
benjie Feb 6, 2017
09fd1f1
How on earth does TypeScript compile this?!
benjie Feb 6, 2017
edd133e
Pass through resolveInfo ready for applying to mutations
benjie Feb 6, 2017
76a9208
Support lookup on mutations
benjie Feb 7, 2017
fb40c7d
Fix type discovery on mutations
benjie Feb 7, 2017
11a0a96
Fix early return on relations
benjie Feb 7, 2017
880e77b
You almost escaped, you little rascal!
benjie Feb 7, 2017
6d1a314
Safety
benjie Feb 7, 2017
55d038e
Fix functions that return JSON or scalars directly
benjie Feb 7, 2017
cd09cc9
You cheeky little...
benjie Feb 7, 2017
2fa1b05
Use raw pgAttribute names (row_id vs id issue)
benjie Feb 7, 2017
e5e4e58
Fix issue with procedures returning same type as parent
benjie Feb 7, 2017
f51c9bc
Allow importing debugPgClient (e.g. in tests)
benjie Feb 7, 2017
f8f68ef
Add fallbacks for computed columns on compound types
benjie Feb 7, 2017
f45e45c
Always fetch primary keys
benjie Feb 7, 2017
63f55a3
If first or last is zero, there's no next/previous page
benjie Feb 7, 2017
9e358cc
Fix discrepencies with hasNextPage/hasPreviousPage
benjie Feb 7, 2017
384da26
Safety
benjie Feb 8, 2017
333f513
Fix computed column output on mutating functions
benjie Feb 8, 2017
d1325c9
Allow cookies from graphiql
benjie Feb 27, 2017
3304343
Handle fallback gracefully
benjie Mar 1, 2017
e9d8035
Change credentials to same-origin
benjie May 22, 2017
387d5f0
Copy fix from master
benjie May 22, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 6 additions & 2 deletions src/graphql/schema/collection/createCollectionGqlType.ts
Expand Up @@ -37,6 +37,7 @@ export default function createCollectionGqlType<TValue> (
primaryKey && [options.nodeIdFieldName, {
description: 'A globally unique identifier. Can be used in various places throughout the system to identify this single value.',
type: new GraphQLNonNull(GraphQLID),
externalFieldNameDependencies: primaryKey._keyTypeFields.map(([fieldName, field]) => field.externalFieldName),
Copy link
Collaborator

Choose a reason for hiding this comment

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

primaryKey._keyTypeFields depends on an internal implementation detail, right? Any reason why we couldn’t just do: externalKeyDependencies: [primaryKey]. This would also work for some relations.

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't know where to grab this from, so just grabbed the nearest available thing. Also what if it's a compound primary key?

Copy link
Contributor

Choose a reason for hiding this comment

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

external describes the database side or is external the graphql side?

resolve: (value: TValue) => idSerde.serialize(collection, value),
}],
],
Expand All @@ -50,6 +51,7 @@ export default function createCollectionGqlType<TValue> (
value: {
description: field.description,
Copy link
Contributor

Choose a reason for hiding this comment

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

missing "," in line 52

Copy link
Member Author

Choose a reason for hiding this comment

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

Huh, good catch. It seems the TypeScript compiler (at least the version I'm using) is hella forgiving. It's also really REALLY hard to figure out where syntax has gone wrong with TypeScript as compared to Babel which is so much more helpful with its errors.

type: gqlType,
externalFieldName: field.externalFieldName,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any reason we can’t have just externalField: field here?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't see why not.

resolve: (value: TValue): mixed => intoGqlOutput(field.getValue(value)),
},
}
Expand Down Expand Up @@ -93,11 +95,13 @@ export default function createCollectionGqlType<TValue> (
],
// We use the config when creating a connection field to inject
// a condition that limits what we select from the paginator.
getPaginatorInput: (headValue: TValue, args: { condition?: { [key: string]: mixed } }) =>
getPaginatorInput: (aliasIdentifier: mixed, args: { condition?: { [key: string]: mixed } }) =>
conditionHelpers.and(
relation.getTailConditionFromHeadValue!(headValue),
relation.getTailConditionFromHeadAlias!(aliasIdentifier),
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we have to handle classicIds here as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that PgRelation::getTailConditionFromHeadAlias does not convert the row_id of _headFieldNames to id?

Copy link
Member Author

Choose a reason for hiding this comment

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

The place to handle it would be here: https://github.com/calebmer/postgraphql/pull/342/files#diff-44b7c98267d37271bb1a324470ae1767R50 - but you're correct - it needs handling

conditionFromGqlInput(args.condition),
),
subquery: true,
relation,
}),
]
}),
Expand Down
Expand Up @@ -93,11 +93,11 @@ function createCollectionPrimaryKeyField <TValue, TKey>(
},
},

async resolve (_source, args, context): Promise<mixed> {
async resolve (_source, args, context, resolveInfo): Promise<mixed> {
const result = idSerde.deserialize(inventory, args[options.nodeIdFieldName] as string)
if (result.collection !== collection) throw new Error(`The provided id is for collection '${result.collection.name}', not the expected collection '${collection.name}'.`)
if (!keyType.isTypeOf(result.keyValue)) throw new Error(`The provided id is not of the correct type.`)
const value = await collectionKey.read!(context, result.keyValue)
const value = await collectionKey.read!(context, result.keyValue, resolveInfo, collectionGqlType)
if (value == null) return
return intoGqlOutput(value)
},
Expand All @@ -123,9 +123,9 @@ function createCollectionKeyField <TValue, TKey>(
return {
type: collectionGqlType,
args: buildObject(inputHelpers.fieldEntries),
async resolve (_source, args, context): Promise<mixed> {
async resolve (_source, args, context, resolveInfo): Promise<mixed> {
const key = inputHelpers.getKeyFromInput(args)
const value = await collectionKey.read!(context, key)
const value = await collectionKey.read!(context, key, resolveInfo, collectionGqlType)
if (value == null) return
return intoGqlOutput(value)
},
Expand Down
Expand Up @@ -46,11 +46,12 @@ export default function createCollectionRelationTailGqlFieldEntries <TSource, TV
: formatName.field(`${headCollection.type.name}-by-${relation.name}`),
{
description: `Reads a single ${scrib.type(headCollectionGqlType)} that is related to this ${scrib.type(collectionGqlType)}.`,
externalFieldNameDependencies: relation._tailFieldNames, // 🔥 Needs to account for classicIds
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to account here for classicIds? The source of a relation is a column != id , it does not seem to address the destination of the relation here.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's possible that your primary key is called "flibbledeedee" and that your "id" column is merely a foreign key.

type: headCollectionGqlType,
async resolve (source: TSource, _args: {}, context: mixed): Promise<mixed> {
async resolve (source: TSource, _args: {}, context: mixed, resolveInfo: mixed): Promise<mixed> {
const value = options.getCollectionValue(source)
const key = relation.getHeadKeyFromTailValue(value)
const headValue = await headCollectionKey.read!(context, key)
const headValue = await headCollectionKey.read!(context, key, resolveInfo, headCollectionGqlType)

if (headValue == null)
return
Expand Down
Expand Up @@ -27,6 +27,7 @@ export default function createCreateCollectionMutationFieldEntry <TValue>(

return [formatName.field(name), createMutationGqlField<TValue>(buildToken, {
name,
relatedGqlType: collectionGqlType,
description: `Creates a single ${scrib.type(collectionGqlType)}.`,

inputFields: [
Expand Down Expand Up @@ -74,7 +75,7 @@ export default function createCreateCollectionMutationFieldEntry <TValue>(
// When we execute we just create a value in the collection after
// transforming the correct input field.
// TODO: test
execute: (context, input) =>
collection.create!(context, inputFromGqlInput(input[inputFieldName])),
execute: (context, input, resolveInfo) =>
collection.create!(context, inputFromGqlInput(input[inputFieldName]), resolveInfo, collectionGqlType),
})]
}
Expand Up @@ -5,6 +5,7 @@ import BuildToken from '../../BuildToken'
import createMutationGqlField from '../../createMutationGqlField'
import createCollectionKeyInputHelpers from '../createCollectionKeyInputHelpers'
import { getDeleteCollectionPayloadGqlType } from './createDeleteCollectionMutationFieldEntry'
import getGqlOutputType from '../../type/getGqlOutputType'

/**
* Creates a delete mutation which will delete a single value from a collection
Expand All @@ -22,13 +23,15 @@ export default function createDeleteCollectionKeyMutationFieldEntry <TValue, TKe
const { collection } = collectionKey
const name = `delete-${collection.type.name}-by-${collectionKey.name}`
const inputHelpers = createCollectionKeyInputHelpers(buildToken, collectionKey)
const { gqlType } = getGqlOutputType(buildToken, collection.type)

return [formatName.field(name), createMutationGqlField<TValue>(buildToken, {
name,
description: `Deletes a single \`${formatName.type(collection.type.name)}\` using a unique key.`,
relatedGqlType: gqlType,
inputFields: inputHelpers.fieldEntries,
payloadType: getDeleteCollectionPayloadGqlType(buildToken, collection),
execute: (context, input) =>
collectionKey.delete!(context, inputHelpers.getKeyFromInput(input)),
execute: (context, input, resolveInfo) =>
collectionKey.delete!(context, inputHelpers.getKeyFromInput(input), resolveInfo, gqlType),
})]
}
Expand Up @@ -6,6 +6,7 @@ import createMutationGqlField from '../../createMutationGqlField'
import createMutationPayloadGqlType from '../../createMutationPayloadGqlType'
import getGqlOutputType from '../../type/getGqlOutputType'
import createCollectionRelationTailGqlFieldEntries from '../createCollectionRelationTailGqlFieldEntries'
import getGqlOutputType from '../../type/getGqlOutputType'

/**
* Creates a delete mutation that uses the primary key of a collection and an
Expand All @@ -26,9 +27,12 @@ export default function createDeleteCollectionMutationFieldEntry <TValue>(
const { options, inventory } = buildToken
const name = `delete-${collection.type.name}`

const { gqlType } = getGqlOutputType(buildToken, collection.type)

return [formatName.field(name), createMutationGqlField<TValue>(buildToken, {
name,
description: `Deletes a single \`${formatName.type(collection.type.name)}\` using its globally unique id.`,
relatedGqlType: gqlType,
inputFields: [
// The only input field we want is the globally unique id which
// corresponds to the primary key of this collection.
Expand All @@ -40,13 +44,15 @@ export default function createDeleteCollectionMutationFieldEntry <TValue>(
payloadType: getDeleteCollectionPayloadGqlType(buildToken, collection),
// Execute by deserializing the id into its component parts and delete a
// value in the collection using that key.
execute: (context, input) => {
execute: (context, input, resolveInfo) => {
const result = idSerde.deserialize(inventory, input[options.nodeIdFieldName] as string)

const { gqlType } = getGqlOutputType(buildToken, collection.type)

if (result.collection !== collection)
throw new Error(`The provided id is for collection '${result.collection.name}', not the expected collection '${collection.name}'.`)

return primaryKey.delete!(context, result.keyValue)
return primaryKey.delete!(context, result.keyValue, resolveInfo, gqlType)
},
})]
}
Expand All @@ -64,6 +70,7 @@ function createDeleteCollectionPayloadGqlType <TValue>(
const { gqlType, intoGqlOutput } = getGqlOutputType(buildToken, new NullableType(collection.type))
return createMutationPayloadGqlType<TValue>(buildToken, {
name: `delete-${collection.type.name}`,
relatedGqlType: gqlType,
outputFields: [
// Add the deleted value as an output field so the user can see the
// object they just deleted.
Expand Down
Expand Up @@ -5,6 +5,7 @@ import BuildToken from '../../BuildToken'
import createMutationGqlField from '../../createMutationGqlField'
import createCollectionKeyInputHelpers from '../createCollectionKeyInputHelpers'
import { getCollectionPatchType, getUpdateCollectionPayloadGqlType } from './createUpdateCollectionMutationFieldEntry'
import getGqlOutputType from '../../type/getGqlOutputType'

/**
* Creates a update mutation which will update a single value from a collection
Expand All @@ -24,10 +25,12 @@ export default function createUpdateCollectionKeyMutationFieldEntry <TValue, TKe
const inputHelpers = createCollectionKeyInputHelpers(buildToken, collectionKey)
const patchFieldName = formatName.field(`${collection.type.name}-patch`)
const { gqlType: patchGqlType, fromGqlInput: patchFromGqlInput } = getCollectionPatchType(buildToken, collection)
const { gqlType } = getGqlOutputType(buildToken, collection.type)

return [formatName.field(name), createMutationGqlField<TValue>(buildToken, {
name,
description: `Updates a single \`${formatName.type(collection.type.name)}\` using a unique key and a patch.`,
relatedGqlType: gqlType,
inputFields: [
// Include all of the fields we need to construct the key value we will
// use to find the single value to update.
Expand All @@ -42,7 +45,7 @@ export default function createUpdateCollectionKeyMutationFieldEntry <TValue, TKe
}],
],
payloadType: getUpdateCollectionPayloadGqlType(buildToken, collection),
execute: (context, input) =>
collectionKey.update!(context, inputHelpers.getKeyFromInput(input), patchFromGqlInput(input[patchFieldName] as {})),
execute: (context, input, resolveInfo) =>
collectionKey.update!(context, inputHelpers.getKeyFromInput(input), patchFromGqlInput(input[patchFieldName] as {}), resolveInfo, gqlType),
})]
}
Expand Up @@ -35,10 +35,12 @@ export default function createUpdateCollectionMutationFieldEntry <TValue>(
const name = `update-${collection.type.name}`
const patchFieldName = formatName.field(`${collection.type.name}-patch`)
const { gqlType: patchGqlType, fromGqlInput: patchFromGqlInput } = getCollectionPatchType(buildToken, collection)
const { gqlType } = getGqlOutputType(buildToken, collection.type)

return [formatName.field(name), createMutationGqlField<TValue>(buildToken, {
name,
description: `Updates a single \`${formatName.type(collection.type.name)}\` using its globally unique id and a patch.`,
relatedGqlType: gqlType,
inputFields: [
// The only input field we want is the globally unique id which
// corresponds to the primary key of this collection.
Expand All @@ -58,13 +60,15 @@ export default function createUpdateCollectionMutationFieldEntry <TValue>(
payloadType: getUpdateCollectionPayloadGqlType(buildToken, collection),
// Execute by deserializing the id into its component parts and update a
// value in the collection using that key.
execute: (context, input) => {
execute: (context, input, resolveInfo) => {
const result = idSerde.deserialize(inventory, input[options.nodeIdFieldName] as string)

const { gqlType } = getGqlOutputType(buildToken, collection.type)

if (result.collection !== collection)
throw new Error(`The provided id is for collection '${result.collection.name}', not the expected collection '${collection.name}'.`)

return primaryKey.update!(context, result.keyValue, patchFromGqlInput(input[patchFieldName] as {}))
return primaryKey.update!(context, result.keyValue, patchFromGqlInput(input[patchFieldName] as {}), resolveInfo, gqlType)
},
})]
}
Expand Down Expand Up @@ -137,6 +141,7 @@ function createUpdateCollectionPayloadGqlType <TValue>(
const { gqlType, intoGqlOutput } = getGqlOutputType(buildToken, new NullableType(collection.type))
return createMutationPayloadGqlType<TValue>(buildToken, {
name: `update-${collection.type.name}`,
relatedGqlType: gqlType,
outputFields: [
// Add the updated value as an output field so the user can see the
// object they just updated.
Expand Down