CockroachDB always uses Timestamptz (at UTC) as TImestamp, confuses migrator.

[gnat]

See: https://www.cockroachlabs.com/docs/stable/timestamp.html

Cockroach always uses Timestamptz (set to +00:00) when Timestamp is specified. This is confusing to piccolo migrations.

gnat@gnat:~/Desktop/piccolo_examples-master/headless_blog_fastapi$ piccolo migrations forwards all

                              BLOG                              
----------------------------------------------------------------
👍 1 migration already complete
🏁 No migrations need to be run

                          SESSION_AUTH                          
----------------------------------------------------------------
👍 1 migration already complete
⏩ 1 migration not yet run
🚀 Running 1 migration:
  - 2019-11-12T20:47:17 [forwards]... The command failed.
expected DEFAULT expression to have type timestamp, but 'current_timestamp() + '01:00:00'' has type timestamptz
For a full stack trace, use --trace

Suggestions? Too new to Piccolo to know what course of action to take in order to make this work smoothly.

Otherwise Cockroach seems to work fairly flawlessly with Piccolo, because of asyncpg.

[dantownsend]

The default for a Timestamp column is this (from here):

class TimestampNow(Default):
    @property
    def postgres(self):
        return "current_timestamp"

    @property
    def sqlite(self):
        return "current_timestamp"

    def python(self):
        return datetime.datetime.now()

You can see that we use current_timestamp. What we should probably have instead is current_timestamp::timestamp, as current_timestamp includes the timezone.

It seems like CockroachDB is slightly stricter than Postgres, because Postgres allows this (it must automatically do some type conversion).

So it's something we can fix in Piccolo.

In the mean time I can push a fix to the piccolo_examples repo, so it uses a Timestamptz column instead of Timestamp, which should work OK with CockroachDB if you like.

[gnat]

Thanks for the quick reply! @dantownsend You rock!

In the mean time I can push a fix to the piccolo_examples repo, so it uses a Timestamptz column instead of Timestamp, which should work OK with CockroachDB if you like.

Might as well if it's not to big of a deal. Every little bit helps to get closer to friction-free Cockroach DB support. Cockroach Labs really needs to be promoting an async ORM and I'd love it to be piccolo.

[dantownsend]

I've updated the piccolo_examples repo. If you pull the latest, and try running the migration, hopefully it'll work.

Might as well if it's not to big of a deal. Every little bit helps to get closer to friction-free Cockroach DB support. Cockroach Labs really needs to be promoting an async ORM and I'd love it to be piccolo.

Yeah, I really need to dig into CockroachDB more - it should really have first class support from Piccolo. The first step is getting the tests / CI running with it.

[gnat]

Btw, here's a quickstart if you wanted to run some tests for yourself:

• Download/extract (single binary): https://binaries.cockroachdb.com/cockroach-v22.1.6.linux-amd64.tgz
• Start Cockroach: ./cockroach start-single-node --insecure --store=node1 --listen-addr=localhost:26257 --http-addr=localhost:8080 --background
• Enter SQL client: ./cockroach sql --insecure
  • Can create database ...; use ...; from here.
• CTRL-C or killall cockroach to send graceful termination signal.

Default account is root without password. Fancy stats dashboard at http://localhost:8080

It's pretty much the easiest database to set up. :wink:

[gnat]

Here's the changeset that worked for me. No changes to headless_blog_fastapi.

https://github.com/piccolo-orm/piccolo/blob/master/piccolo/columns/defaults/timestamp.py Line 24: return f"CURRENT_TIMESTAMP::timestamp + INTERVAL '{interval_string}'" Line 45: return "current_timestamp::timestamp"

All good:

gnat@gnat:~/Desktop/headless_blog_fastapi$ piccolo migrations forwards all --trace

                              BLOG                              
----------------------------------------------------------------
👍 0 migrations already complete
🏁 No migrations need to be run

                          SESSION_AUTH                          
----------------------------------------------------------------
👍 1 migration already complete
⏩ 1 migration not yet run
🚀 Running 1 migration:
  - 2019-11-12T20:47:17 [forwards]... ok! ✔️

                              USER                              
----------------------------------------------------------------
👍 3 migrations already complete
⏩ 3 migrations not yet run
🚀 Running 3 migrations:
  - 2019-11-14T21:52:21 [forwards]... ok! ✔️
  - 2020-06-11T21:38:55 [forwards]... ok! ✔️
  - 2021-04-30T16:14:15 [forwards]... ok! ✔️

                         PICCOLO_ADMIN                          
----------------------------------------------------------------
👍 0 migrations already complete
🏁 No migrations need to be run
gnat@gnat:~/Desktop/headless_blog_fastapi$ 

[gnat]

piccolo-admin running under Cockroach DB.

[sinisaos]

@gnat This is great. Thanks

[sinisaos]

@gnat Can you please close this issue as you have already done all the work in the main Piccolo ORM repo. Thanks.

[gnat]

Yup, thank you! @sinisaos