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

cargo-pgx can generate schemas #441

Merged
merged 46 commits into from
Feb 25, 2022
Merged

cargo-pgx can generate schemas #441

merged 46 commits into from
Feb 25, 2022

Conversation

Hoverbear
Copy link
Contributor

@Hoverbear Hoverbear commented Feb 15, 2022

This PR reworks how we do SQL (again). It amounts to a lot of code movement but very little actual changes.

It does however amount to several API breaks.

What's different?

  • pgx::datum::sql_entity_graph and pgx_utils::sql_entity_graph are merged and present in pgx_utils::sql_entity_graph together.
  • pgx_utils is now exported from pgx at pgx::utils, this is primarily to expose pgx_utils::sql_entity_graph without requiring users have pgx_utils in their Cargo.toml.
  • The sql-generator binary, .cargo/pgx-linker-script.sh and related .cargo/config settings are no longer needed. The old .cargo/config from 0.1.2x is back.
  • pgx_graph_magic!() now expands with __pgx_source_only_sql_mappings and __pgx_typeid_sql_mappings symbols, allowing cargo-pgx to get the appropriate SQL mappings.
  • cargo pgx schema now:
    1. Uses --message-format=json on some cargo invocations and captures that output.
    2. Processes that JSON to find the OUT_DIR used by the relevant pgx_pg_sys build.
    3. It uses that OUT_DIR data to read, generate, then build (with rustc) a 'stub' shared object. (This gets cached!)
    4. dlopens that shared object.
    5. dlopens the extension.
    6. Calls the pgx_utils::sql_entity_graph::PgxSql builders as the sql-generator used to.
  • cargo pgx test and cargo pgx install had several changes to make them more appropriately and cleanly capture and pass build data for this new process.
  • cargo pgx schema, cargo pgx install, and cargo pgx package all had outputs changed.
  • Most sql_graph_entity related functions got a #[doc(hidden)] attribute to clean up their docs.
  • RUST_LOG env vars get correctly passed in pgx test/

UX

Now generating a schema, or packaging/installing/testing an extension looks like so:

image

Users can optionally enable debug or logging still, here's isolated tracing symbols for a cargo pgx schema run where stubs did not exist:

image

And now where they do:

image

Because we use --message-format=json-render-diagnostics Cargo still outputs nice errors:

image

Performance

This PR has a fairly dramatic impact on build performance!

Using this script:

#! /usr/bin/env bash
set -e

git checkout master
cargo install --path cargo-pgx &> /dev/null
pushd pgx-tests/
cargo pgx schema pg14 --features "pg_test" &> /dev/null # Warm up
hyperfine --runs 3 "touch src/lib.rs && cargo pgx schema pg14 --features pg_test"
popd

git checkout wip-no-binary
cargo install --path cargo-pgx &> /dev/null
pushd pgx-tests/
cargo pgx schema pg14 --features "pg_test" &> /dev/null # Warm up
hyperfine --runs 3 "touch src/lib.rs && cargo pgx schema pg14 --features pg_test"
popd

I evaluated both this PR and the latest release:

ana@autonoma:~/git/zombodb/pgx$ ./bencher.sh 
Switched to branch 'master'
Your branch is up to date with 'origin/master'.
Benchmark 1: touch src/lib.rs && cargo pgx schema pg14 --features pg_test
  Time (mean ± σ):     31.261 s ±  0.054 s    [User: 29.487 s, System: 1.806 s]
  Range (min … max):   31.209 s … 31.316 s    3 runs
 
Switched to branch 'wip-no-binary'
Your branch is up to date with 'origin/wip-no-binary'.
Benchmark 1: touch src/lib.rs && cargo pgx schema pg14 --features pg_test
  Time (mean ± σ):     16.842 s ±  0.190 s    [User: 15.796 s, System: 1.089 s]
  Range (min … max):   16.719 s … 17.061 s    3 runs

Caveats

Like before, with the sql-generator and the linker script, we have undefined behavior if the downstream user brazenly tries to call pgx_pg_sys related functionality in something like #[pgx(sql = path::to::my_fn)].

Upgrading

  • Install the new cargo-pgx version and update your pgx related dependencies to the same.
  • Remove .cargo/pgx-linker-script.sh, src/bin/sql-generator.rs.
  • Replace .cargo/config with:
    [build]
    # Postgres symbols won't be available until runtime
    rustflags = ["-C", "link-args=-Wl,-undefined,dynamic_lookup"]
  • In your Cargo.toml:
      [lib]
    + crate-type = ["cdylib"]
    - crate-type = ["cdylib", "rlib"]

Signed-off-by: Ana Hobden <[email protected]>
Signed-off-by: Ana Hobden <[email protected]>
Signed-off-by: Ana Hobden <[email protected]>
@Hoverbear Hoverbear self-assigned this Feb 15, 2022
@Hoverbear
Copy link
Contributor Author

Okay, so, this kinda works?

Signed-off-by: Ana Hobden <[email protected]>
Signed-off-by: Ana Hobden <[email protected]>
Signed-off-by: Ana Hobden <[email protected]>
Signed-off-by: Ana Hobden <[email protected]>
Signed-off-by: Ana Hobden <[email protected]>
Signed-off-by: Ana Hobden <[email protected]>
@Hoverbear Hoverbear changed the title (WIP) cargo-pgx can generate schemas cargo-pgx can generate schemas Feb 22, 2022
@Hoverbear Hoverbear marked this pull request as ready for review February 22, 2022 18:31
@@ -0,0 +1,113 @@
use crate::sql_entity_graph::{pgx_sql::PgxSql, to_sql::ToSqlFn, SqlGraphEntity};
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to be better at using git mv... This is a file that got moved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant