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

Restore MySQL driver and fix ordering problem #858

Merged
merged 31 commits into from
Jun 25, 2020
Merged

Restore MySQL driver and fix ordering problem #858

merged 31 commits into from
Jun 25, 2020

Conversation

jamescheney
Copy link
Contributor

@jamescheney jamescheney commented Jun 22, 2020

This PR restores the MySQL driver, a precondition for #322, and fixes #856 (albeit this could be cleaned up a bit).

Marked as WIP because the way query results are constructed by Sqlite3 is still a little inefficient and could be improved, and I'd like to make progress towards #855 also before merging, unless it turns out to be harder than I think.

  • Avoid unnecessary List.revs in sqlite3 driver and value.ml
  • Basic support for INSERT RETURNING for all supported dbs.
  • Delete remaining dead code

@jamescheney jamescheney marked this pull request as draft June 22, 2020 14:44
jamescheney and others added 11 commits June 22, 2020 15:56
* remove last vestiges of CGI script web mode

* whitespace
) (#843)

* hygienic references to library functions in pattern compilation

* removed commented out dead code
* Fix #154 by checking for ajax request data
As hinted at by a1ffb17, Links was not handling the linearity of the End type correctly. This was because of a very subtle bug in checking linearity of dualised session types.

This patch:

  1. Fixes the duality bug
  2. Ensures `connect` is correct given the asynchronous semantics of `close`
  3. Fixes all examples which mistakenly treated `End` as unrestricted
* fix 754 by passing regular expressions through let insertion and flattening stages

* whitespace

* whitespace

* add genuinely nested query test
* comment out is_query checks in Iteration

* add newly needed query keyword

* tab

* remove relax_query_type_constraint

* remove relax_query_type_constraint

* remove commented out code

* newline

* added test for #835

* tab

* Initial attempt at #841 for discussion

* whitespace argh

* remove dead code
(Bad Simon.)
add -linkall flag which makes mysql plugin happy again
cleanup fix of ordering
@jamescheney
Copy link
Contributor Author

This fixes parts of #856 except for the extension to support RETURNING multiple columns which I leave as a todo.

Something horrible has happened so the PR / branch probably needs to be redone from a clean checkout of master.

@SimonJF
Copy link
Member

SimonJF commented Jun 22, 2020

Regarding something horrible happening, have you tried:

Here's a good workaround. Use the Edit button when viewing the PR in GitHub to change the base branch to something other than master. Then switch it back to master and it will now correctly show only the changes from the most recent commits.

(from the following link; there are also some other solutions)

https://stackoverflow.com/questions/16306012/github-pull-request-showing-commits-that-are-already-in-target-branch

@jamescheney
Copy link
Contributor Author

I seem to have found / remembered the incantation needed to get the branches synced up again, which was git merge --no-ff master - if that doesn't cause problems I'm inclined to leave as is.

@SimonJF
Copy link
Member

SimonJF commented Jun 22, 2020

Ah, good to know!

@jamescheney
Copy link
Contributor Author

So the CI is failing because of an apparent regression on #835...

@jamescheney jamescheney changed the base branch from master to issue-835 June 22, 2020 15:51
@jamescheney jamescheney changed the base branch from issue-835 to master June 22, 2020 15:51
remove relax query type constraint flag
@jamescheney
Copy link
Contributor Author

I take that back, I only thought that because I had a stale build. Looks like I just needed to adjust the factorials example again.

@jamescheney jamescheney linked an issue Jun 22, 2020 that may be closed by this pull request
3 tasks
@jamescheney
Copy link
Contributor Author

I've decided to weaken #855 sufficiently that this fixes it.

add efficient polymorphic append-only buffers

avoid need for extra list reverse in sqlite3

add test that large database results can be obtained (constructing lists whose length > OCaml's stack bound)

fix performance bug in mysql driver
@jamescheney
Copy link
Contributor Author

Now fixed sqlite3 inefficiency and realized that mysql driver suffered from a different problem (whose solution is similar). Previously, we would randomly access the mysql library's result which was apparently slow. Now, instead I load it all into an append-only buffer. Possibly doing this for postgres results would be better and more uniform but this PR is already getting too big.

@jamescheney
Copy link
Contributor Author

The code for handling null integers in mySQL was broken, this is now fixed and I've added a database test that checks that null handling works correctly, as a first step towards #655. This should be ready for review now.

@jamescheney jamescheney marked this pull request as ready for review June 23, 2020 16:42
@jamescheney jamescheney requested review from SimonJF and dhil June 23, 2020 16:43
@jamescheney jamescheney changed the title WIP: Restore MySQL driver and fix ordering problem Restore MySQL driver and fix ordering problem Jun 23, 2020
@jamescheney jamescheney mentioned this pull request Jun 23, 2020
@jamescheney
Copy link
Contributor Author

This should await #859.

@jamescheney
Copy link
Contributor Author

Miraculously, this seems to have commited with 859 and #861, so now that they are merged, will merge this once CI passes if no one stops me.

@jamescheney jamescheney merged commit eda2a17 into master Jun 25, 2020
@jamescheney jamescheney deleted the issue322 branch June 25, 2020 11:17
frank-emrich pushed a commit to frank-emrich/links that referenced this pull request Sep 16, 2020
add -linkall flag which makes mysql plugin happy

make the database tests deterministic

fix inconsistent ordering behavior

add configs for other drivers

cleanup fix of ordering

implement insert returning correctly for mysql, sqlite3

add efficient polymorphic append-only buffers

avoid need for extra list reverse in sqlite3

add test that large database results can be obtained (constructing lists whose length > OCaml's stack bound)

fix performance bug in mysql driver

add test for null integer handling
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants