Updates for Julia 0.7 #40

ararslan · 2018-06-06T00:57:05Z

~~Work in progress, opening now then I'll add commits incrementally.~~

ararslan · 2018-06-06T01:03:51Z

circle.yml

+          mkdir jlmnt
+          hdiutil mount -readonly -mountpoint jlmnt julia.dmg
+          cp -a jlmnt/*.app/Contents/Resources/julia ~/
+          echo 'PATH=$HOME/bin:$PATH' >> $BASH_ENV


I got this from https://discuss.circleci.com/t/how-to-add-a-path-to-path-in-circle-2-0/11554/10, but it doesn't appear to be working properly, hence the failures on Circle.

You need to source bashrc, like in that link

But also I think you could either just not use $PATH, or do an install to whatever directory brew is using

Actually you need that to be `echo 'export PATH=$HOME/bin:$PATH' >> $BASH_ENV

The export is important

Whoooooops. Added export and source, but either I'm still derping or modifying the path isn't the right solution.

Previously I tried symlinking the executable to /usr/local/bin but it couldn't find it there, and I tried symlinking to /usr/bin but got a permissions error, even with sudo.

Might as well just reference Julia by path then?

iamed2 · 2018-06-26T04:21:57Z

src/LibPQ.jl

@@ -3,6 +3,7 @@ module LibPQ
 export status, reset!, execute, clear, fetch!, prepare,
    num_columns, num_rows, num_params, num_affected_rows

+using Compat


Please import specific things rather than everything

iamed2 · 2018-06-26T04:24:00Z

src/parsing.jl

@@ -252,7 +252,7 @@ end
 ## arrays
 # numeric arrays never have double quotes and always use ',' as a separator
 function parse_numeric_array(eltype::Type{T}, str::AbstractString) where T
-    eq_ind = searchindex(str, '=')
+    eq_ind = something(findfirst(isequal('='), str), 0)


Can you just change the eq_ind > 0 check to eq_ind !== nothing and avoid the something?

On 0.6, this returns 0 rather than nothing when there's no match. something is just a more concise way to handle both versions at once.

Ohhh, so this is for cross-version support. Maybe make a comment showing that we can change it when we drop 0.6

iamed2 · 2018-06-26T04:25:26Z

test/runtests.jl

-using Compat: Test
-
-import Compat: @__MODULE__
+using Compat


Please be specific about Compat imports

iamed2 · 2018-06-26T04:27:35Z

src/parsing.jl

    end

    return arr
 end

 function array_size(str)
-    ndims = findfirst(c -> c != '{', str) - 1
+    ndims = something(findfirst(c -> c != '{', str), 0) - 1


Maybe remove the something and throw an error on nothing (like "Invalid PostgreSQL array literal '$str'")

Should we also through an error when this is < 0 on 0.6 then?

Nah keep it as is, I understand better after #40 (comment)

iamed2 · 2018-06-27T16:07:11Z

src/datastreams.jl


    # this should change to be whatever custom pgtype conversion function we invent
-    map!(parameters, values(row)) do val
+    map!(parameters, collect(values(row))) do val


Why collect now? What changed?

Seems that values returns a Tuple rather than an array, which hits a MethodError from map!.

We should make this a loop assigning to parameters instead then. Don't want to allocate an extra vector for each row.

Also add a finalizer for Result that calls `close`. `clear!` has been moved to the Distributed stdlib package as of Julia v0.7. The `cleared` field of the `Result` type has been removed in favor of a general `isopen` method for `Result`s.

iamed2 · 2018-07-04T19:51:59Z

docs/src/index.md

@@ -15,12 +15,12 @@ using LibPQ, DataStreams, NamedTuples
 conn = LibPQ.Connection("dbname=postgres")
 result = execute(conn, "SELECT typname FROM pg_type WHERE oid = 16")
 data = Data.stream!(result, NamedTuple)
-clear!(result)
+close(result)


We can stop recommending this by default now that finalizers exist

iamed2 · 2018-07-04T19:58:22Z

Doctests are failing on 0.7

ararslan · 2018-07-04T20:20:09Z

Fixed.

iamed2 · 2018-07-04T20:39:17Z

docs/src/index.md


 # the same but with parameters
 result = execute(conn, "SELECT typname FROM pg_type WHERE oid = \$1", ["16"])
 data = Data.stream!(result, NamedTuple)
-clear!(result)
+close(result)


You missed this one

We no longer need to recommend explicitly calling `close` on `Result`s and we only want to load NamedTuples on 0.6.

ararslan commented Jun 6, 2018

View reviewed changes

ararslan force-pushed the aa/0.7 branch 4 times, most recently from ed05c16 to f29fb98 Compare June 7, 2018 01:53

iamed2 requested changes Jun 26, 2018

View reviewed changes

ararslan force-pushed the aa/0.7 branch from 14e82c2 to 439ce04 Compare June 26, 2018 19:02

iamed2 reviewed Jun 27, 2018

View reviewed changes

ararslan force-pushed the aa/0.7 branch from 6b90be2 to 8b8d24d Compare June 28, 2018 03:33

ararslan added 2 commits June 28, 2018 12:37

Add 0.7 testing to CI providers

a41ee88

Declare NamedTuples as a test dependency

08f756d

ararslan force-pushed the aa/0.7 branch 3 times, most recently from 9f2cc88 to 38af842 Compare June 28, 2018 23:29

ararslan added 3 commits July 4, 2018 12:04

Fix deprecation warnings on 0.7

c43624a

Fix use of deprecated Memento function

be30362

Use close instead of clear!, add isopen

17e807d

Also add a finalizer for Result that calls `close`. `clear!` has been moved to the Distributed stdlib package as of Julia v0.7. The `cleared` field of the `Result` type has been removed in favor of a general `isopen` method for `Result`s.

ararslan force-pushed the aa/0.7 branch from 54a6a24 to 17e807d Compare July 4, 2018 19:05

ararslan changed the title ~~WIP: Updates for Julia 0.7~~ Updates for Julia 0.7 Jul 4, 2018

iamed2 reviewed Jul 4, 2018

View reviewed changes

Update documentation and Documenter usage

d053c23

We no longer need to recommend explicitly calling `close` on `Result`s and we only want to load NamedTuples on 0.6.

ararslan force-pushed the aa/0.7 branch from 573784c to d053c23 Compare July 4, 2018 20:42

iamed2 approved these changes Jul 4, 2018

View reviewed changes

iamed2 merged commit defa092 into master Jul 4, 2018

iamed2 deleted the aa/0.7 branch July 4, 2018 22:42

This was referenced Jul 4, 2018

Automatically clear results #25

Closed

Heavy memory load when doing a large number of transactions using prepared statements #37

Closed

ararslan mentioned this pull request Jul 4, 2018

UndefVarError:INference not defined on Julia 0.7-alpha #38

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updates for Julia 0.7 #40

Updates for Julia 0.7 #40

ararslan commented Jun 6, 2018 •

edited by iamed2

ararslan Jun 6, 2018

iamed2 Jun 6, 2018

iamed2 Jun 6, 2018

iamed2 Jun 6, 2018

ararslan Jun 6, 2018

iamed2 Jun 6, 2018

iamed2 Jun 26, 2018

iamed2 Jun 26, 2018

ararslan Jun 26, 2018

iamed2 Jun 28, 2018

iamed2 Jun 26, 2018

iamed2 Jun 26, 2018

ararslan Jun 26, 2018

iamed2 Jun 28, 2018

iamed2 Jun 27, 2018

ararslan Jun 27, 2018

iamed2 Jun 27, 2018

iamed2 Jul 4, 2018

iamed2 commented Jul 4, 2018

ararslan commented Jul 4, 2018

iamed2 Jul 4, 2018

ararslan Jul 4, 2018

Updates for Julia 0.7 #40

Updates for Julia 0.7 #40

Conversation

ararslan commented Jun 6, 2018 • edited by iamed2

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iamed2 commented Jul 4, 2018

ararslan commented Jul 4, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ararslan commented Jun 6, 2018 •

edited by iamed2