rmculpepper / sql

Embedding of some of SQL into Racket
33 stars 5 forks source link

Tips for making this work in Typed Racket (with typed/db)? #15

Closed hashimmm closed 3 years ago

hashimmm commented 5 years ago

I'm use this package with the typed/db library. The motivation being that I wish to keep as much of my application in Typed Racket as possible.

So when I pass something constructed using "select" over to untyped db and execute the query, it works just fine. And the value produced by select satisfies the statement? predicate. But if I try passing it over to typed/db and execute, it says it requires a statement and fails.

Is this a bug, or is this intended, or does this have something to do with typed/db being different from db? A glance at the source led me to believe they should be the same. Any clues on how to make this work?

Is this something I should ask on racket-users?

I'm using Racket 7.2. A runnable example looks like this:

#lang typed/racket

(module selector racket
  (require sql)
  (define (get-select)
    (select a #:from b))

  ;; The following prints #t
  (require db)
  (printf "get-select produces statement? ~a~n"
          (statement? (get-select)))

  ;; The following works fine
  ;(define (get-select2)
  ;  (select a #:from b2))
  ;(define conn (sqlite3-connect #:database 'memory))
  ;(query-exec conn "create table b2 (a text);")
  ;(in-query conn (get-select2))

  (provide get-select))

(require typed/db)
(require/typed 'selector
               [get-select (-> Statement)])

(define conn (sqlite3-connect #:database 'memory))
(query-exec conn "create table b (a text);")
(in-query conn (get-select))
hashimmm commented 5 years ago

On slightly closer inspection, it seems these don't match:

typed-racket-more/typed/db/base.rkt

(define-type Statement (U String Prepared-Statement Virtual-Statement Statement-Binding))

db/private/generic/functions.rkt:

(define (statement? x)
  (or (string? x)
      (prepared-statement? x)
      (statement-binding? x)
      (prop:statement? x)))
rmculpepper commented 5 years ago

Yes, that's the problem. I've pushed fixes to the db and typed-racket repos. Your example should run now, but it prints warnings related to wrapping opaque values as type Any.

hashimmm commented 5 years ago

How do I use the github version? I'm assuming I have to do this:

raco pkg update --lookup --catalog https://pkgs.racket-lang.org --clone racket/typed-racket
raco pkg update --lookup --catalog https://pkgs.racket-lang.org --clone racket/db

But after doing that, running my example in DrRacket is still erroring out in the same way. Is there any way to make sure what version of the package DrRacket is using?

hashimmm commented 5 years ago

A bit more info: From DrRacket, if I right-click "db" from (require db), I get an option to open main.rkt, which gives me the newly downloaded main.rkt. From there, if I right-click (require db/base) and open base.rkt, I get the newly downloaded base.rkt. From there, if I right-click statement? and hit "Jump to definition (in other file)", I get the OLD db/private/generic/functions.rkt. If I run locate generic/functions.rkt on my system, I get only a single hit, which is the old one. If I run locate generic/functions2.rkt, I get a single hit, from the new db-lib location. Was I supposed to install some other package? Is there any way to check which collection was provided from which package?

rmculpepper commented 5 years ago

The Typed Racket fix was in the typed-racket-more package. You don't need a clone unless you want to make further changes of your own. Here's the simplest way to get the fixes for 7.2:

raco pkg update --catalog https://pkgs.racket-lang.org db-lib typed-racket-more

You can check the package versions like this:

raco pkg show db-lib typed-racket-more

You want db-lib = de824c6868f8ad05... and typed-racket-more = 468e527be1c3d294...

Parts of the db library, including db/private/generic/functions.rkt, are actually in the Racket base, since raco setup uses SQLite to manage documentation cross-references.

hashimmm commented 5 years ago

Cool, that worked. Thanks!

As for the warning about wrapping opaque values as Any, about which racket additionally says This warning will become an error in a future release, will this require a change in the sql library? I saw this warning when, to workaround this issue on older versions of db-lib and typed-racket-more, I was importing db instead of typed/db and using require/typed for various things, and ended up with this snippet:

(module statement-definer racket
  (require db)
  (require sql)
  (require racket/contract)
  (define stmt? (λ(x) (or (statement? x)
                          (sql-statement? x))))
  (provide (contract-out [stmt? (-> any/c boolean?)])))

(require (only-in typed/racket/unsafe unsafe-require/typed))

(unsafe-require/typed 'statement-definer
                      [#:opaque Statement stmt?])

i.e. I used unsafe-require/typed because my understanding is that the warning means TR thinks statement? might be doing something dodgy but I can inspect the code to figure that it doesn't. Probably.