RollHQ / join-monster-mysql

Improved MySQL dialect for the Join Monster GraphQL to SQL query execution layer
MIT License
1 stars 1 forks source link

Original src before async/await babel transform #1

Open sfarthin opened 6 years ago

sfarthin commented 6 years ago

The source is hard to read with the babel async/await transform applied. Can you provide the source before this transformation? Thanks!

cliedeman commented 6 years ago

I also tried to use this code but it did not work out of the box for me. I converted the code back to es6, replaced some duplicates with join-monster internals and added a $total column.

Gist.

I plan to put it into a repo with some integration tests at some point....

mnpenner commented 6 years ago

@cliedeman Nice! I'm using your implementation now. Did you end up moving it to a repo?

The quote definition is a little lax though. There's a better one in sqlstring:

function quote(str) {
    return '`' + String(str).replace(/`/g, '``') + '`'; 
}

Edit: Actually, neither implementation is working for me. @cliedeman 's is trying to put NaN for the offset when I use an after cursor, and this one (RollHQ) is stuck at 0. Hmm...

mnpenner commented 6 years ago

I only need keyset paging for now, so I attempted to implement that. Here's the gist.

I copied some code from the mariadb implementation. AFAIK the only reason it's not supported is because of the (a,b) > (1,2) syntax? That's silly because it's pretty trivial to unroll [They already implemented it for Oracle, just not MySQL]. Comes out like this:

SELECT
  `branchConn`.`id` AS `id`,
  `branchConn`.`startDate` AS `startDate`,
  `branchConn`.`baseUrl` AS `baseUrl`,
  `branchConn`.`browser` AS `browser`
FROM (
  SELECT `branchConn`.*
  FROM Branches `branchConn`
  WHERE (`branchConn`.`startDate` > 1497048247760 OR (`branchConn`.`startDate` <=> 1497048247760 AND `branchConn`.`id` > X'0d9d964a4a205c94e7a087512237b5bc'))
  ORDER BY `branchConn`.`startDate` ASC, `branchConn`.`id` ASC
  LIMIT 4
) `branchConn`
ORDER BY `branchConn`.`startDate` ASC, `branchConn`.`id` ASC

Not certain why we need the nested query though. Maybe it matters when the query gets more complex...?

cliedeman commented 6 years ago

Hi @mnpenner I am still interested in improving this. I just had a basic pagination use case so I only got that working. Should I create a repo so we can do some collaboration/testing? I have seen some PR's getting merged in join-monster again recently so that would be my end goal. I was also a little wary of this dialect because the nested query is definitely not ideal but I am not sure how else to do it with mysql.

Ciaran

mposborne commented 6 years ago

Hi all - I am very sorry I haven't been able to respond much to this original comment as i've been so busy with other work but i'm getting back to this now. Thank you for the enthusiasm in the project!

To answer the original question, that's my bad sorry. I was just lazy and copied a lot of the code from the original dialects but obviously took the transform code instead of the original. I can probably fix this but it sounds like @cliedeman already has so thank you!

The nested query approach is a requirement, i'm struggling to remember why but i've wondered this myself and I did figure it out at the time. It's not something I have done, it's already part of the join-monster project. I have since made some modifications which improve on this so when I get a moment i'll post them here.

Unfortunately I only needed a simple pagination use case myself so that's all I built into this. Proper keyset paging would be great if someone else wants to add it though?

cliedeman commented 6 years ago

@mposborne on the nested query: Its probably because the default implmentation uses a LATERAL JOIN which is not supported by mysql.

mnpenner commented 6 years ago

The problem I'm having now is that we can't override arrToConnection within the dialect, nor objToCursor. Also, can we even avoid the nested query within the context of handlePaginationAtRoot? It looks like we can only push additional tables into the join list.

This is my first attempt at "keyset" paging but it seems that some of the restrictions that JoinMonster imposes are unnecessary. It's baked into arrToConnection that after you page forward, you can no longer move backwards. objToCursor does not handle Buffers (UUIDs) nicely.

And then pagination in general requires that you perfectly conform (including nullability) to the Relay spec.

I think I'd have to fork the project to get it to behave the way I like.