[REQUEST] dataloader accept information of 'selected fields'
@muddlebee がすでに取り組んでいます。
2020年3月6日 から。
評価
- 難易度
- 5/5
- 見積もり時間
- 1週間以上
- 初心者へのやさしさ
- 25/100
- issue の種類
- 機能追加
- 明瞭さ
- おおむね明確
- 活発さ
- 停滞
- 技術スタック
- graphql, javascript, nodejs
調査の方向性
まず、既存の DataLoader のコンストラクター、load()、loadMany()、バッチ処理、およびキャッシュの動作を確認します。要求されたフィールドをどのように結合するか、またキャッシュされたフィールドのサブセットが後続のロードをどのように満たすかを定義し、その後、バッチ処理と部分的なキャッシュヒットに焦点を当てたテストで、提案する API とキャッシュのセマンティクスを検証します。
索引モデルが issue の本文から書いたものです。
説明
What problem am I trying to solve?
Dataloader currently can solve the problem of collection of data leads to multiple database access.
But batchLoadFn may query unnecessary data from database in order to suit for all kind of resolvers.
Here is my example:
There is a Graph Type called Car and it represents my database table cars.
sql database
create table cars(
id INT,
brand string
)
create table car_trade_records(
id INT,
carId INT,
owner varchar(255),
logDatetime timestamp
)
create table car_accident_records(
id INT,
carId INT,
location varchar(255),
logDatetime timestamp
)
car.graphql
type Car {
id: Int,
brand: String,
lastOwner: String,
lastAccidentLocation: String
}
type Query {
cars(ids: [Int]): [Car]
}
resolvers.js
let resolvers = {
Car :{
latestOwner: async (car, args, context, info) => {
let carRecord = await carLoader.load(car.id)
return carRecord.lastOwner
},
lastAccidentLocation: async (car, args, context, info) => {
let carRecord = await carLoader.load(car.id)
return carRecord.lastAccidentLocation
}
},
Query: {
cars: async (car, args, context, info) => {
return await carLoader.loadMany(args.ids)
}
}
}
the dataloader
// native sql approach
const carLoader = new DataLoader(keys => {
return await mysql.query(`SELECT *,
(SELECT owner FROM car_trade_records trade
WHERE trade.cardId = car.id ORDER BY logDatetime DESC LIMIT 1) AS lastOwner ,
(SELECT location FROM car_accident_records acc
WHERE acc.carId = car.id ORDER BY logDatetime DESC LIMIT 1) AS lastAccidentLocation
FROM car WHERE id IN ?`, keys)
})
// ORM approach. It is sails.js example. ORM usually can get the related table's data easily.
const carLoader = new DataLoader(keys => {
return (await Car.find({where:{id: keys}})
.populate('carOwners', {limit: 1})
.populate('carAccidents', {limit: 1})
).map( c => {
c.lastOwner = c.carOwners.owner
c.lastAccidentLocation = c.carAccidents.location
return c
})
})
If the graphql query doesn't involve both lastOwner or lastAccidentLocation fields at same time, there will be unnecessary database query cost because the SQL query will make unnecessary table joins/selection.
If the car_trade_records and car_accident_records are huge table or with some complex data manipulation, the unnecessary cost will be significant.
Query examples:
# Example 1: No unnecessary cost
cars(ids: [1,2,3,4]) {
id,
brand,
lastOwner,
lastAccidentLocation
}
# Example 2: selecting unnecessary data from the table 'car_accident_record'
cars(ids: [1,2,3,4]) {
id,
brand,
lastOwner
}
# Example 3: selecting unnecessary data from the table 'car_trade_record'
cars(ids: [1,2,3,4]) {
id,
brand,
lastAccidentLocation
}
# Example 4: selecting unnecessary data from both two tables
cars(ids: [1,2,3,4]) {
id,
brand
}
Someone might think that we can 'join other tables' on demand (in the resolvers) like below. But there will be additional database queries. If 4 car records are selected, 8 (4 * 2=8) more sql queries will be made.
// using sails.js ORM to get data from database
let resolvers = {
Car: {
latestOwner: async (car, args, context, info) => {
let carRecord = await carLoader.load(car.id)
return (await CarTradeRecord.find({
where: {
carId: car.id
},
sort: [{'logDatetime': 'desc'}],
limit: 1
}
)).owner
},
lastAccidentLocation: async (car, args, context, info) => {
let carRecord = await carLoader.load(car.id)
return (await CarAccidentRecord.find({
where: {
carId: car.id
},
sort: [{'logDatetime': 'desc'}],
limit: 1
}
)).location
}
},
Query: {
cars: async (car, args, context, info) => {
return await carLoader.loadMany(args.ids)
}
}
}
const carLoader = new DataLoader(keys => {
return await Car.find({ where: {id: keys} })
})
The new feature I'd like to have
If dataloader can accept more infomation about what attributes will be selected, it can reduce unnecessary cost in querying data.
Proposed changes of DataLoader functions:
load(key, fields)
key: A key value to load.fields: (optional) An array of field to load. Default value = []
loadMany(keys, fields)
keys: An array of key values to load.fields: (optional) An array of field to load. Default value = []
new DataLoader(batchLoadFn [, options])
batchLoadFn: function(keys, fields)keys: an array of keysfields: an array of field that are being queried.
For load() and loadMany(), they will return a cached value only if both the key and fields matches those key and fields in batchLoadFn().
Example 1
// assume that cached value is never expired.
const loader = new DataLoader((keys, fields)=> {
console.log('keys:', keys)
console.log('fields:', fields)
return batchLoadImpl(keys, fields)
})
(async ()=> {
console.log('No cache')
let a1 = loader.load(1, ['field1', 'field2'])
let a2 = loader.load(2, ['field2', 'field3'])
await a1 && await a2
console.log('All hit cache')
let b1 = loader.load(1, ['field2'])
let b2 = loader.load(1, ['field3'])
let b3 = loader.load(2, ['field1'])
let b4 = loader.load(2, ['field2'])
let b5 = loader.load(2)
// value can be taken from cached if the requested fields are the part of the cached fields
await b1 && await b2 && await b3 && await b4 && await b5
console.log('Some hit cache')
let c1 = loader.load(1, ['field1', 'field4'])
let c2 = loader.load(2, ['field1', 'field2', 'field3'])
//value with key 2 can be taken from cached
await c1 && await c2
console.log('Different cache even with same key')
let d1 = loader.load(2, ['field1'])
let d2 = loader.load(2, ['field1', 'field4'])
console.log('d1 is from cache?', (await d1) === (await c2)) // get from cache?
console.log('d2 is from cache?', (await d2) === (await c2)) // get from cache?
})()
Expected console outputs:
No cache
keys: [1, 2]
fields: ['field1', 'field2', 'field3']
All hit cache
Some hit cache
keys: [1]
fields: ['field1', 'field4']
Different cache even with same key
keys: [2]
fields: ['field1, 'field4']
d1 is from cache? true
d2 is from cache? false
Example 2: demostrate the real case
// native sql approach
const carLoader = new DataLoader( (keys, fields) => {
let subqueries = []
if(fields.includes('latestOwner')){
subqueries.push(`(SELECT owner FROM car_trade_records trade
WHERE trade.cardId = car.id ORDER BY logDatetime DESC LIMIT 1) AS lastOwner`)
}
if(fields.includes('lastAccidentLocation')){
subqueries.push(`(SELECT location FROM car_accident_records acc
WHERE acc.carId = car.id ORDER BY logDatetime DESC LIMIT 1) AS lastAccidentLocation`)
}
return await mysql.query(`SELECT *, ${subqueries.join(',')} FROM car WHERE id IN ?`, keys)
})
let resolvers = {
Car :{
latestOwner: async (car, args, context, info) => {
// info.fieldName: 'latestOwner'
let carRecord = await carLoader.load(car.id, [info.fieldName])
return carRecord.lastOwner
},
lastAccidentLocation: async (car, args, context, info) => {
// info.fieldName: 'lastAccidentLocation'
let carRecord = await carLoader.load(car.id, [info.fieldName])
return carRecord.lastAccidentLocation
}
},
Query: {
cars: async (car, args, context, info) => {
//determine the necessary fields of the Car Entity in graphql query
let involvedFields = info.fieldNodes.map(node => node.name.value)
return await carLoader.loadMany(args.ids, involvedFields)
}
}
}
If it is implemented, no matter what fields are selected to Type Car, only one single sql query will be made.
- 主要言語
- JavaScript
- スター
- 13.4k
- フォーク
- 522
- PR マージ指標
- 30日以内にマージされた PR はありません
環境構築
- Dockerfile・Docker Compose ファイルなし
- プルリクエストのテンプレートなし
- コントリビューションガイドを読む
はじめの一歩
- issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
- 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
- リポジトリをフォークし、ブランチを切って変更します。
- issue 番号を参照したプルリクエストを送ります。
graphql/dataloader のほかの issue
-
[BUG] `maxBatchSize: NaN` is accepted and silently disables batching対応中かも @RaphaelFakhri が 2 日前に担当しました。 オープン
難易度 2/5 1〜3時間 初心者へのやさしさ 82/100
graphql/dataloader#405 ·
-
Failed batch evicts a replacement cache entry after clear/prime対応中かも @Claire-ovo が 11 日前に担当しました。 オープン
難易度 4/5 3〜5日 初心者へのやさしさ 57/100
graphql/dataloader#403 ·
-
enhancement
難易度 5/5 1週間以上 初心者へのやさしさ 38/100
graphql/dataloader#393 ·
-
enhancement
難易度 5/5 1週間以上 初心者へのやさしさ 25/100
graphql/dataloader#382 ·
-
enhancement
難易度 4/5 3〜5日 初心者へのやさしさ 35/100
graphql/dataloader#379 · コメント 2 件 · リアクション 5 件 ·
graphql/dataloader の issue をすべて見る
似ている issue
-
難易度 2/5 1〜3時間 初心者へのやさしさ 76/100
Automattic/node-canvas#2633 ·
-
難易度 2/5 1〜3時間 初心者へのやさしさ 78/100
capricorn86/happy-dom#2594 ·
メンテナーはふだん 1 日以内に返信
-
難易度 2/5 1〜3時間 初心者へのやさしさ 78/100
-
難易度 2/5 1〜3時間 初心者へのやさしさ 79/100
microsoft/monaco-editor#5575 ·
メンテナーはふだん 1 日以内に返信
-
難易度 2/5 1〜3時間 初心者へのやさしさ 84/100
メンテナーはふだん 1 日以内に返信