From fe58529d43188a4c08f7d2c354008381b227967c Mon Sep 17 00:00:00 2001 From: Marc-Antoine Enriquez Date: Wed, 25 Dec 2019 03:29:24 +0100 Subject: [PATCH 01/57] Use correct data type in unix --- pg-extend/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pg-extend/src/lib.rs b/pg-extend/src/lib.rs index f4156afa..6882fc94 100644 --- a/pg-extend/src/lib.rs +++ b/pg-extend/src/lib.rs @@ -142,7 +142,7 @@ unsafe fn pg_sys_longjmp(_buf: *mut pg_sys::_JBTYPE, _value: ::std::os::raw::c_i } #[cfg(unix)] -unsafe fn pg_sys_longjmp(_buf: *mut pg_sys::_JBTYPE, _value: ::std::os::raw::c_int) { +unsafe fn pg_sys_longjmp(_buf: *mut pg_sys::__jmp_buf_tag, _value: ::std::os::raw::c_int) { pg_sys::siglongjmp(_buf, _value); } From e55b2d8cb955906439314eafc5c8baccb5a98c84 Mon Sep 17 00:00:00 2001 From: Benjamin Fry Date: Thu, 26 Dec 2019 17:30:00 -0800 Subject: [PATCH 02/57] fixup build script to properly support features --- Makefile.toml | 160 ++------------------------------- integration-tests/tests/fdw.rs | 4 +- pg-extend/Cargo.toml | 10 ++- pg-extend/Makefile.toml | 8 +- pg-extend/src/lib.rs | 56 ++---------- pg-extend/src/pg_fdw.rs | 2 +- 6 files changed, 29 insertions(+), 211 deletions(-) diff --git a/Makefile.toml b/Makefile.toml index 3a933015..8bd11b90 100644 --- a/Makefile.toml +++ b/Makefile.toml @@ -24,6 +24,8 @@ CARGO_MAKE_KCOV_DOWNLOAD_DIRECTORY = "${TARGET_DIR}/kcov-dl" CARGO_MAKE_KCOV_VERSION = "37" # This should match the 12.1 settings +ALL_FEATURES = "--all-features" +VER_FEATURES = "--features=postgres-12" PG_VERSION = "12.1" PG_PORT = "5444" @@ -39,6 +41,7 @@ PG_LOGPATH = "${TARGET_DIR}/postgres-${PG_VERSION}.log" PATH = "${PG_BIN_DIR}:${PATH}" [env.v10] +VER_FEATURES = "--features=postgres-10" PG_VERSION = "10.11" PG_PORT = "5442" # port is incremented by the major version, 5432 + ${PG_VERSION} @@ -54,6 +57,7 @@ PG_LOGPATH = "${TARGET_DIR}/postgres-${PG_VERSION}.log" PATH = "${PG_BIN_DIR}:${PATH}" [env.v11] +VER_FEATURES = "--features=postgres-11" PG_VERSION = "11.6" PG_PORT = "5443" # port is incremented by the major version, 5432 + ${PG_VERSION} @@ -69,6 +73,7 @@ PG_LOGPATH = "${TARGET_DIR}/postgres-${PG_VERSION}.log" PATH = "${PG_BIN_DIR}:${PATH}" [env.v12] +VER_FEATURES = "--features=postgres-12" PG_VERSION = "12.1" PG_PORT = "5444" # port is incremented by the major version, 5432 + ${PG_VERSION} @@ -164,76 +169,6 @@ condition_script = ["if cargo with --version ; then exit 1 ; else exit 0 ; fi"] command = "cargo" args = ["install", "cargo-with", "--git=https://github.com/bluejekyll/cargo-with.git", "--branch=master"] -[tasks.install-kcov] -description = "Installs the kcov coverage tool" -workspace = false -script_runner = "bash" -install_script = [ -''' -KCOV_INSTALLATION_DIRECTORY="" -KCOV_BINARY_DIRECTORY="" -if [ -n "${CARGO_MAKE_KCOV_INSTALLATION_DIRECTORY}" ]; then - mkdir -p ${CARGO_MAKE_KCOV_INSTALLATION_DIRECTORY} - cd ${CARGO_MAKE_KCOV_INSTALLATION_DIRECTORY} - KCOV_INSTALLATION_DIRECTORY="$(pwd)/" - cd - - echo "Kcov Installation Directory: ${KCOV_INSTALLATION_DIRECTORY}" - KCOV_BINARY_DIRECTORY="${KCOV_INSTALLATION_DIRECTORY}/build/src/" - echo "Kcov Binary Directory: ${KCOV_BINARY_DIRECTORY}" -fi - -if ${KCOV_BINARY_DIRECTORY}kcov --version ; then exit 0 ; fi - -# get help info to fetch all supported command line arguments -KCOV_HELP_INFO=`${KCOV_BINARY_DIRECTORY}kcov --help` || true - -# check needed arguments are supported, else install -if [[ $KCOV_HELP_INFO != *"--include-pattern"* ]] || [[ $KCOV_HELP_INFO != *"--exclude-line"* ]] || [[ $KCOV_HELP_INFO != *"--exclude-region"* ]]; then - # check we are on a supported platform - if [ "$(uname)" == "Linux" ]; then - if [ "$(grep -Ei 'debian|buntu|mint' /etc/*release)" ]; then - echo "Installing/Upgrading kcov..." - sudo apt-get update || true - sudo apt-get install -y libcurl4-openssl-dev libelf-dev libdw-dev cmake gcc binutils-dev - fi - elif [ "$(uname)" == "Darwin" ]; then - for brew_install in zlib bash cmake pkgconfig wget ; do - if brew info ${brew_install} | grep "Not installed" ; then - brew install ${brew_install} - else - echo "skipping ${brew_install} already installed" - fi - done - fi - - mkdir -p ${CARGO_MAKE_KCOV_DOWNLOAD_DIRECTORY} - cd ${CARGO_MAKE_KCOV_DOWNLOAD_DIRECTORY} - KCOV_DOWNLOAD_DIRECTORY=$(pwd) - - rm -rf kcov-${CARGO_MAKE_KCOV_VERSION:?} - rm -f v${CARGO_MAKE_KCOV_VERSION}.zip - - wget https://github.com/SimonKagstrom/kcov/archive/v${CARGO_MAKE_KCOV_VERSION}.zip - unzip v${CARGO_MAKE_KCOV_VERSION}.zip - cd kcov-${CARGO_MAKE_KCOV_VERSION} - mkdir -p build - cd ./build - cmake .. - make - - # if custom installation directory, leave kcov as local - if [ -n "${CARGO_MAKE_KCOV_INSTALLATION_DIRECTORY}" ]; then - cd ${KCOV_DOWNLOAD_DIRECTORY}/kcov-${CARGO_MAKE_KCOV_VERSION} - mv ./* ${KCOV_INSTALLATION_DIRECTORY} - else - sudo make install - cd ../.. - rm -rf kcov-${CARGO_MAKE_KCOV_VERSION} - fi -fi -''' -] - ## ## Postgres operations @@ -412,7 +347,7 @@ args = ["test", "--jobs=1", "--lib", "--examples", "--tests", "--bins", "@@remov description = "Run the clippy linter on all crates" dependencies = ["clean", "install-postgres"] command = "cargo" -args = ["clippy", "--lib", "--examples", "--tests", "--bins", "--all-features", "--", "-D", "warnings"] +args = ["clippy", "--lib", "--examples", "--tests", "--bins", "${ALL_FEATURES}", "--", "-D", "warnings"] [tasks.build-bench] description = "Check that all benchmarks compile" @@ -443,102 +378,21 @@ run_task = "all" [tasks.default-features] description = "Run all with default features" dependencies = ["install-openssl"] -workspace = false env = { FEATURES = "" } run_task = { name = "all", fork = true } [tasks.no-default-features] description = "Run all with --no-default-features" dependencies = ["install-openssl"] -workspace = false env = { FEATURES = "--no-default-features" } run_task = { name = "all", fork = true } [tasks.all-features] description = "Run all with --all-features" dependencies = ["install-openssl"] -workspace = false -# can't currently test --all-features on pg-extend because it would enable all versions, which won't compile -env = { FEATURES = "--all-features", CARGO_MAKE_WORKSPACE_SKIP_MEMBERS = "pg-extend" } +env = { FEATURES = "${ALL_FEATURES}" } run_task = { name = "all", fork = true } -## -## Coverage -## - -[tasks.coverage-kcov] -description = "Installs (if missing) and runs coverage using kcov (not supported on windows)" -windows_alias = "empty" -mac_alias = "empty" # TODO: mac works, but is so slow it hangs on some tests... -dependencies = ["install-with", "install-kcov"] -script_runner = "bash" -env = { CARGO_MAKE_KCOV_INCLUDE_PATTERN = "${CARGO_MAKE_WORKING_DIRECTORY}/src/", CARGO_MAKE_COVERAGE_REPORT_DIRECTORY = "${TARGET_DIR}/coverage", CARGO_MAKE_WITH_KCOV_ARGS = "${TDNS_WITH_KCOV_ARGS}" } -script = [ -''' -set -e - -echo "Working Directory: ${CARGO_MAKE_WORKING_DIRECTORY}" - -KCOV_BINARY_DIRECTORY="" -if [ -n "${CARGO_MAKE_KCOV_INSTALLATION_DIRECTORY}" ]; then - cd ${CARGO_MAKE_KCOV_INSTALLATION_DIRECTORY} - KCOV_INSTALLATION_DIRECTORY="$(pwd)/" - cd - - echo "Kcov Installation Directory: ${KCOV_INSTALLATION_DIRECTORY}" - KCOV_BINARY_DIRECTORY="${KCOV_INSTALLATION_DIRECTORY}/build/src/" - echo "Kcov Binary Directory: ${KCOV_BINARY_DIRECTORY}" -fi - -TARGET_DIRECTORY="target/coverage/${CARGO_MAKE_CRATE_FS_NAME}" -if [ -n "$CARGO_MAKE_COVERAGE_REPORT_DIRECTORY" ]; then - TARGET_DIRECTORY="$CARGO_MAKE_COVERAGE_REPORT_DIRECTORY/${CARGO_MAKE_CRATE_FS_NAME}" - mkdir -p ${TARGET_DIRECTORY} -else - mkdir -p ${TARGET_DIRECTORY} -fi - -BINARY_DIRECTORY=target/debug -if [ -n "$CARGO_MAKE_WORKSPACE_TARGET_DIRECTORY" ]; then - BINARY_DIRECTORY="${CARGO_MAKE_WORKSPACE_TARGET_DIRECTORY}/debug" -fi - -KCOV_EXCLUDE_LINE_ARG="" -if [ -n "$CARGO_MAKE_KCOV_EXCLUDE_LINE" ]; then - KCOV_EXCLUDE_LINE_ARG="--exclude-line=${CARGO_MAKE_KCOV_EXCLUDE_LINE}" -fi - -KCOV_EXCLUDE_REGION_ARG="" -if [ -n "$CARGO_MAKE_KCOV_EXCLUDE_REGION" ]; then - KCOV_EXCLUDE_REGION_ARG="--exclude-region=${CARGO_MAKE_KCOV_EXCLUDE_REGION}" -fi - -cd ${CARGO_MAKE_WORKING_DIRECTORY} -echo "Working director: ${PWD}" -echo "Running tests from directory: ${BINARY_DIRECTORY}" - -# Evaluate variables that may be in the expression -# This allows us to do double expansion on a non-variable second expansion -CARGO_MAKE_TEST_COVERAGE_BINARY_FILTER_REGEX="$(sh -c "echo \"${CARGO_MAKE_TEST_COVERAGE_BINARY_FILTER}\"")" -echo "Test binary filter regex: ${CARGO_MAKE_TEST_COVERAGE_BINARY_FILTER_REGEX}" - -## Setup the env for the kcov script -KCOV=${CARGO_MAKE_WORKSPACE_WORKING_DIRECTORY:?}/scripts/kcov.sh - -export KCOV_BINARY=${KCOV_BINARY_DIRECTORY:?}/kcov -export KCOV_TARGET_DIRECTORY=${TARGET_DIRECTORY:?} -export KCOV_INCLUDE_PATTERN=${CARGO_MAKE_KCOV_INCLUDE_PATTERN:?} -export KCOV_EXCLUDE_LINE_ARG -export KCOV_EXCLUDE_REGION_ARG - -echo "Running coverage for ${CARGO_MAKE_CRATE_FS_NAME} ${CARGO_MAKE_WITH_KCOV_ARGS} ${FEATURES}" -cargo with "${KCOV:?} {bin} {args}" -- test ${CARGO_MAKE_WITH_KCOV_ARGS} ${FEATURES} || true -''' -] - -[tasks.coverage] -description = "Run coverage reports on all crates" -run_task = "coverage-kcov" - ## ## publishing ## diff --git a/integration-tests/tests/fdw.rs b/integration-tests/tests/fdw.rs index 2e00abd7..fc2903dd 100644 --- a/integration-tests/tests/fdw.rs +++ b/integration-tests/tests/fdw.rs @@ -7,15 +7,15 @@ // FDW tests disabled because it's broken with PostgreSQL 11+. // See See https://github.com/bluejekyll/pg-extend-rs/issues/49 -#![cfg(fdw_is_broken)] extern crate integration_tests; use integration_tests::*; #[test] +#[ignore] // this test is currently broken fn test_fdw() { - test_in_db("fdw", |conn| { + test_in_db("fdw", |mut conn| { conn.batch_execute( " DROP SERVER IF EXISTS df CASCADE; diff --git a/pg-extend/Cargo.toml b/pg-extend/Cargo.toml index 857c0a54..67e792b0 100644 --- a/pg-extend/Cargo.toml +++ b/pg-extend/Cargo.toml @@ -18,10 +18,18 @@ build = "build.rs" [features] default = [] + +# Enable Foreign Data wrappers support fdw = [] + +# We use feature flags to dictate which sets of PG features we support. +# The build.rs script will set a feature flag rustc, but this does not enable the dependencies. +# For that, build scripts that want to explcitly enable supported features in each +# version, should pass the feature flag explicity. +# Each of these should list all features supported by that version of PG. postgres-9 = ["fdw"] postgres-10 = ["fdw"] -postgres-11 = [] +postgres-11 = ["fdw"] postgres-12 = [] [dependencies] diff --git a/pg-extend/Makefile.toml b/pg-extend/Makefile.toml index f59a9064..bb32c87d 100644 --- a/pg-extend/Makefile.toml +++ b/pg-extend/Makefile.toml @@ -8,10 +8,4 @@ private = true ## Feature profiles [env] CARGO_MAKE_EXTEND_WORKSPACE_MAKEFILE = "true" -TDNS_WITH_KCOV_ARGS = "--bins --tests --examples" - -[tasks.clippy] -description = "Run the clippy linter on all crates" -dependencies = ["clean", "install-postgres"] -command = "cargo" -args = ["clippy", "--lib", "--examples", "--tests", "--bins", "--features=fdw", "--", "-D", "warnings"] +ALL_FEATURES = "${VER_FEATURES}" diff --git a/pg-extend/src/lib.rs b/pg-extend/src/lib.rs index f722ef35..416a27ad 100644 --- a/pg-extend/src/lib.rs +++ b/pg-extend/src/lib.rs @@ -141,14 +141,20 @@ cfg_if::cfg_if! { unsafe fn pg_sys_longjmp(_buf: *mut pg_sys::_JBTYPE, _value: ::std::os::raw::c_int) { pg_sys::longjmp(_buf, _value); } + + type SigjmpBuf = pg_sys::jmp_buf; } else if #[cfg(target_os = "macos")] { unsafe fn pg_sys_longjmp(_buf: *mut c_int, _value: ::std::os::raw::c_int) { pg_sys::siglongjmp(_buf, _value); } + + type SigjmpBuf = pg_sys::sigjmp_buf; } else if #[cfg(unix)] { unsafe fn pg_sys_longjmp(_buf: *mut pg_sys::__jmp_buf_tag, _value: ::std::os::raw::c_int) { pg_sys::siglongjmp(_buf, _value); } + + type SigjmpBuf = pg_sys::sigjmp_buf; } } @@ -160,16 +166,14 @@ cfg_if::cfg_if! { /// this is already handled. /// /// See the man pages for info on setjmp http://man7.org/linux/man-pages/man3/setjmp.3.html -#[cfg(unix)] #[inline(never)] pub(crate) unsafe fn guard_pg R>(f: F) -> R { // setup the check protection - let original_exception_stack: *mut pg_sys::sigjmp_buf = pg_sys::PG_exception_stack; - let mut local_exception_stack: mem::MaybeUninit = - mem::MaybeUninit::uninit(); + let original_exception_stack: *mut SigjmpBuf = pg_sys::PG_exception_stack; + let mut local_exception_stack: mem::MaybeUninit = mem::MaybeUninit::uninit(); let jumped = pg_sys::sigsetjmp( // grab a mutable reference, cast to a mutabl pointr, then case to the expected erased pointer type - local_exception_stack.as_mut_ptr() as *mut pg_sys::sigjmp_buf as *mut _, + local_exception_stack.as_mut_ptr() as *mut SigjmpBuf as *mut _, 1, ); // now that we have the local_exception_stack, we set that for any PG longjmps... @@ -196,48 +200,6 @@ pub(crate) unsafe fn guard_pg R>(f: F) -> R { result } -/// Provides a barrier between Rust and Postgres' usage of the C set/longjmp -/// -/// In the case of a longjmp being caught, this will convert that to a panic. For this to work -/// properly, there must be a Rust panic handler (see crate::register_panic_handler).PanicContext -/// If the `pg_exern` attribute macro is used for exposing Rust functions to Postgres, then -/// this is already handled. -/// -/// See the man pages for info on setjmp http://man7.org/linux/man-pages/man3/setjmp.3.html -#[cfg(windows)] -#[inline(never)] -pub(crate) unsafe fn guard_pg R>(f: F) -> R { - // setup the check protection - let original_exception_stack: *mut pg_sys::jmp_buf = pg_sys::PG_exception_stack; - let mut local_exception_stack: mem::MaybeUninit = mem::MaybeUninit::uninit(); - let jumped = pg_sys::_setjmp( - // grab a mutable reference, cast to a mutabl pointr, then case to the expected erased pointer type - local_exception_stack.as_mut_ptr() as *mut pg_sys::jmp_buf as *mut _, - ); - // now that we have the local_exception_stack, we set that for any PG longjmps... - - if jumped != 0 { - notice!("PG longjmped: {}", jumped); - pg_sys::PG_exception_stack = original_exception_stack; - - // The C Panicked!, handling control to Rust Panic handler - compiler_fence(Ordering::SeqCst); - panic!(JumpContext { jump_value: jumped }); - } - - // replace the exception stack with ours to jump to the above point - pg_sys::PG_exception_stack = local_exception_stack.as_mut_ptr() as *mut _; - - // enforce that the setjmp is not reordered, though that's probably unlikely... - compiler_fence(Ordering::SeqCst); - let result = f(); - - compiler_fence(Ordering::SeqCst); - pg_sys::PG_exception_stack = original_exception_stack; - - result -} - /// auto generate function to output a SQL create statement for the function /// /// Until concat_ident! stabilizes, this requires the name to passed with the appended sctring diff --git a/pg-extend/src/pg_fdw.rs b/pg-extend/src/pg_fdw.rs index dae1c8d6..b9d79714 100644 --- a/pg-extend/src/pg_fdw.rs +++ b/pg-extend/src/pg_fdw.rs @@ -6,7 +6,7 @@ // FDW on PostgreSQL 11+ is not supported. :( // If anyone tries to enable "fdw" feature with newer Postgres, throw error. -#![cfg(not(any(feature = "postgres-11", feature = "postgres-12")))] +#![cfg(not(feature = "postgres-12"))] #![cfg(feature = "fdw")] use std::boxed::Box; From 1cdfccd292796f28fb0259532e0035d9d8285952 Mon Sep 17 00:00:00 2001 From: Benjamin Fry Date: Thu, 26 Dec 2019 17:54:26 -0800 Subject: [PATCH 03/57] cleanup Makefile, reduce duplicate definitions --- Makefile.toml | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/Makefile.toml b/Makefile.toml index 8bd11b90..74a27dc7 100644 --- a/Makefile.toml +++ b/Makefile.toml @@ -10,6 +10,9 @@ [config] skip_core_tasks = true +# Default to v12 +additional_profiles = ["v12"] + [config.modify_core_tasks] # if true, all core tasks are set to private (default false) private = true @@ -23,22 +26,8 @@ CARGO_MAKE_KCOV_INSTALLATION_DIRECTORY = "${TARGET_DIR}/kcov" CARGO_MAKE_KCOV_DOWNLOAD_DIRECTORY = "${TARGET_DIR}/kcov-dl" CARGO_MAKE_KCOV_VERSION = "37" -# This should match the 12.1 settings +# This can be overriden (e.g. in pg-extend crate) to specify a more limited set of features ALL_FEATURES = "--all-features" -VER_FEATURES = "--features=postgres-12" -PG_VERSION = "12.1" -PG_PORT = "5444" - -PG_DIR = "${TARGET_DIR}/postgres" -PG_DL_DIR = "${PG_DIR}" -PG_BUILD_DIR = "${PG_DIR}/postgres_build_${PG_VERSION}" -PG_INSTALL_DIR = "${PG_DIR}/postgres_${PG_VERSION}" -PG_BIN_DIR = "${PG_INSTALL_DIR}/bin" -PG_DB_DIR = "${TARGET_DIR}/postgres_db_${PG_VERSION}" -POSTGRES_TEST_DB = "pg_extend_rs_test_db" -PG_LOGPATH = "${TARGET_DIR}/postgres-${PG_VERSION}.log" - -PATH = "${PG_BIN_DIR}:${PATH}" [env.v10] VER_FEATURES = "--features=postgres-10" From 5a164c4df096d687e98b4d91b09a21f495602980 Mon Sep 17 00:00:00 2001 From: Benjamin Fry Date: Thu, 26 Dec 2019 22:48:35 -0800 Subject: [PATCH 04/57] revert sgjmp changes --- integration-tests/tests/fdw.rs | 5 ++- pg-extend/Cargo.toml | 1 - pg-extend/src/lib.rs | 56 ++++++++++++++++++++++++++++------ pg-extend/src/pg_fdw.rs | 1 + 4 files changed, 50 insertions(+), 13 deletions(-) diff --git a/integration-tests/tests/fdw.rs b/integration-tests/tests/fdw.rs index fc2903dd..4b82fe85 100644 --- a/integration-tests/tests/fdw.rs +++ b/integration-tests/tests/fdw.rs @@ -5,13 +5,12 @@ // http://opensource.org/licenses/MIT>, at your option. This file may not be // copied, modified, or distributed except according to those terms. -// FDW tests disabled because it's broken with PostgreSQL 11+. -// See See https://github.com/bluejekyll/pg-extend-rs/issues/49 - extern crate integration_tests; use integration_tests::*; +// FDW tests disabled because it's broken with PostgreSQL 11+. +// See See https://github.com/bluejekyll/pg-extend-rs/issues/49 #[test] #[ignore] // this test is currently broken fn test_fdw() { diff --git a/pg-extend/Cargo.toml b/pg-extend/Cargo.toml index 67e792b0..e1ea6e81 100644 --- a/pg-extend/Cargo.toml +++ b/pg-extend/Cargo.toml @@ -21,7 +21,6 @@ default = [] # Enable Foreign Data wrappers support fdw = [] - # We use feature flags to dictate which sets of PG features we support. # The build.rs script will set a feature flag rustc, but this does not enable the dependencies. # For that, build scripts that want to explcitly enable supported features in each diff --git a/pg-extend/src/lib.rs b/pg-extend/src/lib.rs index 416a27ad..f722ef35 100644 --- a/pg-extend/src/lib.rs +++ b/pg-extend/src/lib.rs @@ -141,20 +141,14 @@ cfg_if::cfg_if! { unsafe fn pg_sys_longjmp(_buf: *mut pg_sys::_JBTYPE, _value: ::std::os::raw::c_int) { pg_sys::longjmp(_buf, _value); } - - type SigjmpBuf = pg_sys::jmp_buf; } else if #[cfg(target_os = "macos")] { unsafe fn pg_sys_longjmp(_buf: *mut c_int, _value: ::std::os::raw::c_int) { pg_sys::siglongjmp(_buf, _value); } - - type SigjmpBuf = pg_sys::sigjmp_buf; } else if #[cfg(unix)] { unsafe fn pg_sys_longjmp(_buf: *mut pg_sys::__jmp_buf_tag, _value: ::std::os::raw::c_int) { pg_sys::siglongjmp(_buf, _value); } - - type SigjmpBuf = pg_sys::sigjmp_buf; } } @@ -166,14 +160,16 @@ cfg_if::cfg_if! { /// this is already handled. /// /// See the man pages for info on setjmp http://man7.org/linux/man-pages/man3/setjmp.3.html +#[cfg(unix)] #[inline(never)] pub(crate) unsafe fn guard_pg R>(f: F) -> R { // setup the check protection - let original_exception_stack: *mut SigjmpBuf = pg_sys::PG_exception_stack; - let mut local_exception_stack: mem::MaybeUninit = mem::MaybeUninit::uninit(); + let original_exception_stack: *mut pg_sys::sigjmp_buf = pg_sys::PG_exception_stack; + let mut local_exception_stack: mem::MaybeUninit = + mem::MaybeUninit::uninit(); let jumped = pg_sys::sigsetjmp( // grab a mutable reference, cast to a mutabl pointr, then case to the expected erased pointer type - local_exception_stack.as_mut_ptr() as *mut SigjmpBuf as *mut _, + local_exception_stack.as_mut_ptr() as *mut pg_sys::sigjmp_buf as *mut _, 1, ); // now that we have the local_exception_stack, we set that for any PG longjmps... @@ -200,6 +196,48 @@ pub(crate) unsafe fn guard_pg R>(f: F) -> R { result } +/// Provides a barrier between Rust and Postgres' usage of the C set/longjmp +/// +/// In the case of a longjmp being caught, this will convert that to a panic. For this to work +/// properly, there must be a Rust panic handler (see crate::register_panic_handler).PanicContext +/// If the `pg_exern` attribute macro is used for exposing Rust functions to Postgres, then +/// this is already handled. +/// +/// See the man pages for info on setjmp http://man7.org/linux/man-pages/man3/setjmp.3.html +#[cfg(windows)] +#[inline(never)] +pub(crate) unsafe fn guard_pg R>(f: F) -> R { + // setup the check protection + let original_exception_stack: *mut pg_sys::jmp_buf = pg_sys::PG_exception_stack; + let mut local_exception_stack: mem::MaybeUninit = mem::MaybeUninit::uninit(); + let jumped = pg_sys::_setjmp( + // grab a mutable reference, cast to a mutabl pointr, then case to the expected erased pointer type + local_exception_stack.as_mut_ptr() as *mut pg_sys::jmp_buf as *mut _, + ); + // now that we have the local_exception_stack, we set that for any PG longjmps... + + if jumped != 0 { + notice!("PG longjmped: {}", jumped); + pg_sys::PG_exception_stack = original_exception_stack; + + // The C Panicked!, handling control to Rust Panic handler + compiler_fence(Ordering::SeqCst); + panic!(JumpContext { jump_value: jumped }); + } + + // replace the exception stack with ours to jump to the above point + pg_sys::PG_exception_stack = local_exception_stack.as_mut_ptr() as *mut _; + + // enforce that the setjmp is not reordered, though that's probably unlikely... + compiler_fence(Ordering::SeqCst); + let result = f(); + + compiler_fence(Ordering::SeqCst); + pg_sys::PG_exception_stack = original_exception_stack; + + result +} + /// auto generate function to output a SQL create statement for the function /// /// Until concat_ident! stabilizes, this requires the name to passed with the appended sctring diff --git a/pg-extend/src/pg_fdw.rs b/pg-extend/src/pg_fdw.rs index b9d79714..9af4c29d 100644 --- a/pg-extend/src/pg_fdw.rs +++ b/pg-extend/src/pg_fdw.rs @@ -254,6 +254,7 @@ impl ForeignWrapper { match CStr::from_ptr((*ptr_value).defname).to_str() { Ok(key) => { + #[allow(clippy::cast_ptr_alignment)] let arg = (*((*ptr_value).arg as *mut pg_sys::Value)).val.str; let value = match CStr::from_ptr(arg).to_str() { Ok(v) => v.into(), From 9090d45ebec23bb006b0cf48c34d0668c0d10d54 Mon Sep 17 00:00:00 2001 From: Benjamin Fry Date: Thu, 26 Dec 2019 23:22:44 -0800 Subject: [PATCH 05/57] test dedupped environment variables --- Makefile.toml | 40 +++------------------------------------- 1 file changed, 3 insertions(+), 37 deletions(-) diff --git a/Makefile.toml b/Makefile.toml index 74a27dc7..6cee3154 100644 --- a/Makefile.toml +++ b/Makefile.toml @@ -19,6 +19,9 @@ private = true ## General environment configuration [env] +v10 = { PG_VERSION = "10.11", PG_PORT = "5442", VER_FEATURES = "--features=postgres-10" } +v11 = { PG_VERSION = "11.6", PG_PORT = "5443", VER_FEATURES = "--features=postgres-11" } +v12 = { PG_VERSION = "12.1", PG_PORT = "5444", VER_FEATURES = "--features=postgres-12" } TARGET_DIR = "${CARGO_MAKE_WORKSPACE_WORKING_DIRECTORY}/target" CARGO_MAKE_WORKSPACE_TARGET_DIRECTORY = "${TARGET_DIR}" CARGO_MAKE_EXTEND_WORKSPACE_MAKEFILE = "true" @@ -29,43 +32,6 @@ CARGO_MAKE_KCOV_VERSION = "37" # This can be overriden (e.g. in pg-extend crate) to specify a more limited set of features ALL_FEATURES = "--all-features" -[env.v10] -VER_FEATURES = "--features=postgres-10" -PG_VERSION = "10.11" -PG_PORT = "5442" # port is incremented by the major version, 5432 + ${PG_VERSION} - -PG_DIR = "${TARGET_DIR}/postgres" -PG_DL_DIR = "${PG_DIR}" -PG_BUILD_DIR = "${PG_DIR}/postgres_build_${PG_VERSION}" -PG_INSTALL_DIR = "${PG_DIR}/postgres_${PG_VERSION}" -PG_BIN_DIR = "${PG_INSTALL_DIR}/bin" -PG_DB_DIR = "${TARGET_DIR}/postgres_db_${PG_VERSION}" -POSTGRES_TEST_DB = "pg_extend_rs_test_db" -PG_LOGPATH = "${TARGET_DIR}/postgres-${PG_VERSION}.log" - -PATH = "${PG_BIN_DIR}:${PATH}" - -[env.v11] -VER_FEATURES = "--features=postgres-11" -PG_VERSION = "11.6" -PG_PORT = "5443" # port is incremented by the major version, 5432 + ${PG_VERSION} - -PG_DIR = "${TARGET_DIR}/postgres" -PG_DL_DIR = "${PG_DIR}" -PG_BUILD_DIR = "${PG_DIR}/postgres_build_${PG_VERSION}" -PG_INSTALL_DIR = "${PG_DIR}/postgres_${PG_VERSION}" -PG_BIN_DIR = "${PG_INSTALL_DIR}/bin" -PG_DB_DIR = "${TARGET_DIR}/postgres_db_${PG_VERSION}" -POSTGRES_TEST_DB = "pg_extend_rs_test_db" -PG_LOGPATH = "${TARGET_DIR}/postgres-${PG_VERSION}.log" - -PATH = "${PG_BIN_DIR}:${PATH}" - -[env.v12] -VER_FEATURES = "--features=postgres-12" -PG_VERSION = "12.1" -PG_PORT = "5444" # port is incremented by the major version, 5432 + ${PG_VERSION} - PG_DIR = "${TARGET_DIR}/postgres" PG_DL_DIR = "${PG_DIR}" PG_BUILD_DIR = "${PG_DIR}/postgres_build_${PG_VERSION}" From 9acc1c9b1667485ff8b56cc7ba463b1f3991f141 Mon Sep 17 00:00:00 2001 From: Benjamin Fry Date: Fri, 27 Dec 2019 01:01:37 -0800 Subject: [PATCH 06/57] use decoded variables --- Makefile.toml | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/Makefile.toml b/Makefile.toml index 6cee3154..9c3c8ba8 100644 --- a/Makefile.toml +++ b/Makefile.toml @@ -10,18 +10,12 @@ [config] skip_core_tasks = true -# Default to v12 -additional_profiles = ["v12"] - [config.modify_core_tasks] # if true, all core tasks are set to private (default false) private = true ## General environment configuration [env] -v10 = { PG_VERSION = "10.11", PG_PORT = "5442", VER_FEATURES = "--features=postgres-10" } -v11 = { PG_VERSION = "11.6", PG_PORT = "5443", VER_FEATURES = "--features=postgres-11" } -v12 = { PG_VERSION = "12.1", PG_PORT = "5444", VER_FEATURES = "--features=postgres-12" } TARGET_DIR = "${CARGO_MAKE_WORKSPACE_WORKING_DIRECTORY}/target" CARGO_MAKE_WORKSPACE_TARGET_DIRECTORY = "${TARGET_DIR}" CARGO_MAKE_EXTEND_WORKSPACE_MAKEFILE = "true" @@ -32,6 +26,11 @@ CARGO_MAKE_KCOV_VERSION = "37" # This can be overriden (e.g. in pg-extend crate) to specify a more limited set of features ALL_FEATURES = "--all-features" +# Defaults are all for postgres version 12 +PG_VERSION = { source = "${CARGO_MAKE_PROFILE}", default_value = "12.1", mapping = { v10 = "10.11", v11 = "11.6" }} +PG_PORT = { source = "${CARGO_MAKE_PROFILE}", default_value = "5444", mapping = { v10 = "5442", v11 = "5443" }} +VER_FEATURES = { source = "${CARGO_MAKE_PROFILE}", default_value = "--features=postgres-12", mapping = { v10 = "--features=postgres-10", v11 = "--features=postgres-11" }} + PG_DIR = "${TARGET_DIR}/postgres" PG_DL_DIR = "${PG_DIR}" PG_BUILD_DIR = "${PG_DIR}/postgres_build_${PG_VERSION}" From 1dbce1079c7688099bea3ca8ba35ebd3440d1a4d Mon Sep 17 00:00:00 2001 From: Benjamin Fry Date: Fri, 27 Dec 2019 01:12:02 -0800 Subject: [PATCH 07/57] cleanliness should wait for postgres builds --- .github/workflows/test.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index f1f46989..df7dd571 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -110,6 +110,7 @@ jobs: cleanliness: name: cleanliness runs-on: ubuntu-latest + needs: postgres env: version: v11 steps: From 3a7513f2dfd48a09e6606796ebe7ccdbaa93d569 Mon Sep 17 00:00:00 2001 From: Benjamin Fry Date: Fri, 27 Dec 2019 09:04:43 -0800 Subject: [PATCH 08/57] restart PG between tests --- Makefile.toml | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Makefile.toml b/Makefile.toml index 9c3c8ba8..17b01b7c 100644 --- a/Makefile.toml +++ b/Makefile.toml @@ -290,13 +290,17 @@ dependencies = ["install-postgres"] command = "cargo" args = ["build", "--lib", "--bins", "@@remove-empty(FEATURES)"] -[tasks.test] -description = "Run tests all the crates" +[tasks.test-inner] +description = "Run tests on all the crates, without restarting PG" dependencies = ["pg-create-db"] env = { POSTGRES_PORT = "${PG_PORT}" } command = "cargo" args = ["test", "--jobs=1", "--lib", "--examples", "--tests", "--bins", "@@remove-empty(FEATURES)"] +[tasks.test] +description = "Run tests on all the crates, restarts and stops PG" +dependencies = ["pg-stop", "pg-start", "test-inner", "pg-stop"] + [tasks.clippy] description = "Run the clippy linter on all crates" dependencies = ["clean", "install-postgres"] From 7e6c36fa391f83ed8bd20302676e563832218c38 Mon Sep 17 00:00:00 2001 From: Benjamin Fry Date: Fri, 27 Dec 2019 09:17:59 -0800 Subject: [PATCH 09/57] remove deps-cache, was too large to cache --- .github/workflows/test.yml | 9 --------- 1 file changed, 9 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index df7dd571..657bfea8 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -94,15 +94,6 @@ jobs: restore-keys: | ${{ runner.os }}-postgres-${{ matrix.version }}-${{ hashFiles('**/Makefile.toml') }} - - name: target/debug/deps cache - uses: actions/cache@v1 - with: - path: target/debug/deps - key: ${{ runner.os }}-deps-${{ hashFiles('**/Cargo.toml') }}-${{ hashFiles('**/Cargo.lock') }} - restore-keys: | - ${{ runner.os }}-deps-${{ hashFiles('**/Cargo.toml') }} - ${{ runner.os }}-deps - - name: cargo make matrix.feature run: cargo make -p ${{ matrix.version }} ${{ matrix.feature }} From 66b329e9f74c2e037e2013c7ea21fe96d1f5cf41 Mon Sep 17 00:00:00 2001 From: Benjamin Fry Date: Fri, 27 Dec 2019 09:48:08 -0800 Subject: [PATCH 10/57] check connectivity after start --- Makefile.toml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Makefile.toml b/Makefile.toml index 17b01b7c..abb7fb4e 100644 --- a/Makefile.toml +++ b/Makefile.toml @@ -155,8 +155,9 @@ if ${PG_BIN_DIR}/pg_isready -p ${PG_PORT} ; then exit 0 ; fi echo "Starting postgres ${PG_DB_DIR:?}" mkdir -p ${PG_DB_DIR:?} -${PG_BIN_DIR}/pg_ctl restart -D ${PG_DB_DIR:?} -l ${PG_LOGPATH} -o "-p ${PG_PORT:?}" +${PG_BIN_DIR}/pg_ctl start -D ${PG_DB_DIR:?} -l ${PG_LOGPATH} -o "-p ${PG_PORT:?}" ${PG_BIN_DIR}/pg_isready -p ${PG_PORT} -t 5 +${PG_BIN_DIR}/psql postgres -p ${PG_PORT} -o /dev/null -c "SELECT 1" # check the connection works ''' ] From a07e53ad0adf5148855cbec1923fd6ea0fb37700 Mon Sep 17 00:00:00 2001 From: Benjamin Fry Date: Fri, 27 Dec 2019 11:07:55 -0800 Subject: [PATCH 11/57] try to delay after file copies in tests --- integration-tests/src/lib.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/integration-tests/src/lib.rs b/integration-tests/src/lib.rs index 3bd05451..af8cd1f1 100644 --- a/integration-tests/src/lib.rs +++ b/integration-tests/src/lib.rs @@ -147,6 +147,8 @@ pub fn run_create_stmts(bin_path: &PathBuf, lib_path: &PathBuf) { if let Err(err) = result { error = Some(err); } else { + // FIXME: this sleep helps all the tests pass, PG is finding files not ready?? + std::thread::sleep(std::time::Duration::from_millis(100)); return; } } @@ -175,12 +177,16 @@ pub fn copy_to_tempdir(path: &Path, lib_path: PathBuf) -> PathBuf { ) }) .unwrap(); + + // FIXME: this sleep helps all the tests pass, PG is finding files not ready?? + std::thread::sleep(std::time::Duration::from_millis(100)); + tmplib } pub fn test_in_db(lib_name: &str, test: F) { - // FIXME: this sleep helps all the tests pass - std::thread::sleep(std::time::Duration::from_secs(1)); + // FIXME: this sleep helps all the tests pass, PG is finding files not ready?? + std::thread::sleep(std::time::Duration::from_millis(100)); println!("test_in_db: {}", lib_name); let bin_path = build_bin(lib_name).expect("failed to build stmt binary"); From dfc491372767d71d6dd289c8c3731a34f1b424eb Mon Sep 17 00:00:00 2001 From: Benjamin Fry Date: Fri, 27 Dec 2019 14:30:32 -0800 Subject: [PATCH 12/57] add some logging information on failure --- Makefile.toml | 31 +- integration-tests/postgresql.conf | 661 ++++++++++++++++++++++++++++++ 2 files changed, 685 insertions(+), 7 deletions(-) create mode 100644 integration-tests/postgresql.conf diff --git a/Makefile.toml b/Makefile.toml index abb7fb4e..e029432e 100644 --- a/Makefile.toml +++ b/Makefile.toml @@ -9,6 +9,7 @@ [config] skip_core_tasks = true +on_error_task = "on_error" [config.modify_core_tasks] # if true, all core tasks are set to private (default false) @@ -48,23 +49,24 @@ PATH = "${PG_BIN_DIR}:${PATH}" [tasks.install-openssl] description = "Installs OpenSSL on Windows" -env = { OPENSSL_VERSION = "1_1_1d", OPENSSL_DIR = "${CARGO_MAKE_WORKING_DIRECTORY}\\target\\OpenSSL" } +workspace = false +env = { OPENSSL_VERSION = "1_1_1d", OPENSSL_DIR = "${CARGO_MAKE_WORKSPACE_WORKING_DIRECTORY}\\target\\OpenSSL" } condition = { platforms = ["windows"], files_not_exist = ["${OPENSSL_DIR}"] } script_runner = "powershell" script_extension = "ps1" script = [ ''' -mkdir ${env:CARGO_MAKE_WORKING_DIRECTORY}\\target -mkdir ${env:CARGO_MAKE_WORKING_DIRECTORY}\\target\OpenSSL -Invoke-WebRequest -URI "http://slproweb.com/download/Win64OpenSSL-${env:OPENSSL_VERSION}.exe" -OutFile "${env:CARGO_MAKE_WORKING_DIRECTORY}\target\OpenSSL.exe" -Start-Process -FilePath "${env:CARGO_MAKE_WORKING_DIRECTORY}\target\OpenSSL.exe" -ArgumentList "/SILENT /VERYSILENT /SP- /DIR=${env:OPENSSL_DIR}" +mkdir ${env:CARGO_MAKE_WORKSPACE_WORKING_DIRECTORY}\\target +mkdir ${env:CARGO_MAKE_WORKSPACE_WORKING_DIRECTORY}\\target\OpenSSL +Invoke-WebRequest -URI "http://slproweb.com/download/Win64OpenSSL-${env:OPENSSL_VERSION}.exe" -OutFile "${env:CARGO_MAKE_WORKSPACE_WORKING_DIRECTORY}\target\OpenSSL.exe" +Start-Process -FilePath "${env:CARGO_MAKE_WORKSPACE_WORKING_DIRECTORY}\target\OpenSSL.exe" -ArgumentList "/SILENT /VERYSILENT /SP- /DIR=${env:OPENSSL_DIR}" Invoke-WebRequest "https://curl.haxx.se/ca/cacert.pem" -O "${env:OPENSSL_DIR}\cacert.pem" ''' ] [tasks.install-postgres] -workspace = false description = "Installs Postgres" +workspace = false windows_alias = "empty" script_runner = "@shell" script = [ @@ -155,7 +157,11 @@ if ${PG_BIN_DIR}/pg_isready -p ${PG_PORT} ; then exit 0 ; fi echo "Starting postgres ${PG_DB_DIR:?}" mkdir -p ${PG_DB_DIR:?} -${PG_BIN_DIR}/pg_ctl start -D ${PG_DB_DIR:?} -l ${PG_LOGPATH} -o "-p ${PG_PORT:?}" + +cp ${CARGO_MAKE_WORKSPACE_WORKING_DIRECTORY:?}/integration-tests/postgresql.conf ${PG_DB_DIR:?}/ +mv ${PG_LOGPATH:?} ${PG_LOGPATH}.bak || true + +${PG_BIN_DIR}/pg_ctl start -D ${PG_DB_DIR:?} -l ${PG_LOGPATH:?} -o "-p ${PG_PORT:?}" ${PG_BIN_DIR}/pg_isready -p ${PG_PORT} -t 5 ${PG_BIN_DIR}/psql postgres -p ${PG_PORT} -o /dev/null -c "SELECT 1" # check the connection works ''' @@ -330,6 +336,17 @@ dependencies = ["check", "build", "test"] description = "Run the all task" run_task = "all" +[tasks.on_error] +description = "Dumps addition information to the CLI on failure" +workspace = false +script_runner = "@shell" +script = [ +''' +echo "!!!!Dumping PG log after failure!!!!" +cat ${PG_LOGPATH} || true +''' +] + ## ## All feature testing builds ## diff --git a/integration-tests/postgresql.conf b/integration-tests/postgresql.conf new file mode 100644 index 00000000..bc4da121 --- /dev/null +++ b/integration-tests/postgresql.conf @@ -0,0 +1,661 @@ +# ----------------------------- +# PostgreSQL configuration file +# ----------------------------- +# +# This file consists of lines of the form: +# +# name = value +# +# (The "=" is optional.) Whitespace may be used. Comments are introduced with +# "#" anywhere on a line. The complete list of parameter names and allowed +# values can be found in the PostgreSQL documentation. +# +# The commented-out settings shown in this file represent the default values. +# Re-commenting a setting is NOT sufficient to revert it to the default value; +# you need to reload the server. +# +# This file is read on server startup and when the server receives a SIGHUP +# signal. If you edit the file on a running system, you have to SIGHUP the +# server for the changes to take effect, run "pg_ctl reload", or execute +# "SELECT pg_reload_conf()". Some parameters, which are marked below, +# require a server shutdown and restart to take effect. +# +# Any parameter can also be given as a command-line option to the server, e.g., +# "postgres -c log_connections=on". Some parameters can be changed at run time +# with the "SET" SQL command. +# +# Memory units: kB = kilobytes Time units: ms = milliseconds +# MB = megabytes s = seconds +# GB = gigabytes min = minutes +# TB = terabytes h = hours +# d = days + + +#------------------------------------------------------------------------------ +# FILE LOCATIONS +#------------------------------------------------------------------------------ + +# The default values of these variables are driven from the -D command-line +# option or PGDATA environment variable, represented here as ConfigDir. + +#data_directory = 'ConfigDir' # use data in another directory + # (change requires restart) +#hba_file = 'ConfigDir/pg_hba.conf' # host-based authentication file + # (change requires restart) +#ident_file = 'ConfigDir/pg_ident.conf' # ident configuration file + # (change requires restart) + +# If external_pid_file is not explicitly set, no extra PID file is written. +#external_pid_file = '' # write an extra PID file + # (change requires restart) + + +#------------------------------------------------------------------------------ +# CONNECTIONS AND AUTHENTICATION +#------------------------------------------------------------------------------ + +# - Connection Settings - + +#listen_addresses = 'localhost' # what IP address(es) to listen on; + # comma-separated list of addresses; + # defaults to 'localhost'; use '*' for all + # (change requires restart) +#port = 5442 # (change requires restart) +max_connections = 100 # (change requires restart) +#superuser_reserved_connections = 3 # (change requires restart) +#unix_socket_directories = '/tmp' # comma-separated list of directories + # (change requires restart) +#unix_socket_group = '' # (change requires restart) +#unix_socket_permissions = 0777 # begin with 0 to use octal notation + # (change requires restart) +#bonjour = off # advertise server via Bonjour + # (change requires restart) +#bonjour_name = '' # defaults to the computer name + # (change requires restart) + +# - Security and Authentication - + +#authentication_timeout = 1min # 1s-600s +#ssl = off +#ssl_ciphers = 'HIGH:MEDIUM:+3DES:!aNULL' # allowed SSL ciphers +#ssl_prefer_server_ciphers = on +#ssl_ecdh_curve = 'prime256v1' +#ssl_dh_params_file = '' +#ssl_cert_file = 'server.crt' +#ssl_key_file = 'server.key' +#ssl_ca_file = '' +#ssl_crl_file = '' +#password_encryption = md5 # md5 or scram-sha-256 +#db_user_namespace = off +#row_security = on + +# GSSAPI using Kerberos +#krb_server_keyfile = '' +#krb_caseins_users = off + +# - TCP Keepalives - +# see "man 7 tcp" for details + +#tcp_keepalives_idle = 0 # TCP_KEEPIDLE, in seconds; + # 0 selects the system default +#tcp_keepalives_interval = 0 # TCP_KEEPINTVL, in seconds; + # 0 selects the system default +#tcp_keepalives_count = 0 # TCP_KEEPCNT; + # 0 selects the system default + + +#------------------------------------------------------------------------------ +# RESOURCE USAGE (except WAL) +#------------------------------------------------------------------------------ + +# - Memory - + +shared_buffers = 128MB # min 128kB + # (change requires restart) +#huge_pages = try # on, off, or try + # (change requires restart) +#temp_buffers = 8MB # min 800kB +#max_prepared_transactions = 0 # zero disables the feature + # (change requires restart) +# Caution: it is not advisable to set max_prepared_transactions nonzero unless +# you actively intend to use prepared transactions. +#work_mem = 4MB # min 64kB +#maintenance_work_mem = 64MB # min 1MB +#replacement_sort_tuples = 150000 # limits use of replacement selection sort +#autovacuum_work_mem = -1 # min 1MB, or -1 to use maintenance_work_mem +#max_stack_depth = 2MB # min 100kB +dynamic_shared_memory_type = posix # the default is the first option + # supported by the operating system: + # posix + # sysv + # windows + # mmap + # use none to disable dynamic shared memory + # (change requires restart) + +# - Disk - + +#temp_file_limit = -1 # limits per-process temp file space + # in kB, or -1 for no limit + +# - Kernel Resource Usage - + +#max_files_per_process = 1000 # min 25 + # (change requires restart) +#shared_preload_libraries = '' # (change requires restart) + +# - Cost-Based Vacuum Delay - + +#vacuum_cost_delay = 0 # 0-100 milliseconds +#vacuum_cost_page_hit = 1 # 0-10000 credits +#vacuum_cost_page_miss = 10 # 0-10000 credits +#vacuum_cost_page_dirty = 20 # 0-10000 credits +#vacuum_cost_limit = 200 # 1-10000 credits + +# - Background Writer - + +#bgwriter_delay = 200ms # 10-10000ms between rounds +#bgwriter_lru_maxpages = 100 # 0-1000 max buffers written/round +#bgwriter_lru_multiplier = 2.0 # 0-10.0 multiplier on buffers scanned/round +#bgwriter_flush_after = 0 # measured in pages, 0 disables + +# - Asynchronous Behavior - + +#effective_io_concurrency = 0 # 1-1000; 0 disables prefetching +#max_worker_processes = 8 # (change requires restart) +#max_parallel_workers_per_gather = 2 # taken from max_parallel_workers +#max_parallel_workers = 8 # maximum number of max_worker_processes that + # can be used in parallel queries +#old_snapshot_threshold = -1 # 1min-60d; -1 disables; 0 is immediate + # (change requires restart) +#backend_flush_after = 0 # measured in pages, 0 disables + + +#------------------------------------------------------------------------------ +# WRITE AHEAD LOG +#------------------------------------------------------------------------------ + +# - Settings - + +#wal_level = replica # minimal, replica, or logical + # (change requires restart) +#fsync = on # flush data to disk for crash safety + # (turning this off can cause + # unrecoverable data corruption) +#synchronous_commit = on # synchronization level; + # off, local, remote_write, remote_apply, or on +#wal_sync_method = fsync # the default is the first option + # supported by the operating system: + # open_datasync + # fdatasync (default on Linux) + # fsync + # fsync_writethrough + # open_sync +#full_page_writes = on # recover from partial page writes +#wal_compression = off # enable compression of full-page writes +#wal_log_hints = off # also do full page writes of non-critical updates + # (change requires restart) +#wal_buffers = -1 # min 32kB, -1 sets based on shared_buffers + # (change requires restart) +#wal_writer_delay = 200ms # 1-10000 milliseconds +#wal_writer_flush_after = 1MB # measured in pages, 0 disables + +#commit_delay = 0 # range 0-100000, in microseconds +#commit_siblings = 5 # range 1-1000 + +# - Checkpoints - + +#checkpoint_timeout = 5min # range 30s-1d +#max_wal_size = 1GB +#min_wal_size = 80MB +#checkpoint_completion_target = 0.5 # checkpoint target duration, 0.0 - 1.0 +#checkpoint_flush_after = 0 # measured in pages, 0 disables +#checkpoint_warning = 30s # 0 disables + +# - Archiving - + +#archive_mode = off # enables archiving; off, on, or always + # (change requires restart) +#archive_command = '' # command to use to archive a logfile segment + # placeholders: %p = path of file to archive + # %f = file name only + # e.g. 'test ! -f /mnt/server/archivedir/%f && cp %p /mnt/server/archivedir/%f' +#archive_timeout = 0 # force a logfile segment switch after this + # number of seconds; 0 disables + + +#------------------------------------------------------------------------------ +# REPLICATION +#------------------------------------------------------------------------------ + +# - Sending Server(s) - + +# Set these on the master and on any standby that will send replication data. + +#max_wal_senders = 10 # max number of walsender processes + # (change requires restart) +#wal_keep_segments = 0 # in logfile segments, 16MB each; 0 disables +#wal_sender_timeout = 60s # in milliseconds; 0 disables + +#max_replication_slots = 10 # max number of replication slots + # (change requires restart) +#track_commit_timestamp = off # collect timestamp of transaction commit + # (change requires restart) + +# - Master Server - + +# These settings are ignored on a standby server. + +#synchronous_standby_names = '' # standby servers that provide sync rep + # method to choose sync standbys, number of sync standbys, + # and comma-separated list of application_name + # from standby(s); '*' = all +#vacuum_defer_cleanup_age = 0 # number of xacts by which cleanup is delayed + +# - Standby Servers - + +# These settings are ignored on a master server. + +#hot_standby = on # "off" disallows queries during recovery + # (change requires restart) +#max_standby_archive_delay = 30s # max delay before canceling queries + # when reading WAL from archive; + # -1 allows indefinite delay +#max_standby_streaming_delay = 30s # max delay before canceling queries + # when reading streaming WAL; + # -1 allows indefinite delay +#wal_receiver_status_interval = 10s # send replies at least this often + # 0 disables +#hot_standby_feedback = off # send info from standby to prevent + # query conflicts +#wal_receiver_timeout = 60s # time that receiver waits for + # communication from master + # in milliseconds; 0 disables +#wal_retrieve_retry_interval = 5s # time to wait before retrying to + # retrieve WAL after a failed attempt + +# - Subscribers - + +# These settings are ignored on a publisher. + +#max_logical_replication_workers = 4 # taken from max_worker_processes + # (change requires restart) +#max_sync_workers_per_subscription = 2 # taken from max_logical_replication_workers + + +#------------------------------------------------------------------------------ +# QUERY TUNING +#------------------------------------------------------------------------------ + +# - Planner Method Configuration - + +#enable_bitmapscan = on +#enable_hashagg = on +#enable_hashjoin = on +#enable_indexscan = on +#enable_indexonlyscan = on +#enable_material = on +#enable_mergejoin = on +#enable_nestloop = on +#enable_seqscan = on +#enable_sort = on +#enable_tidscan = on + +# - Planner Cost Constants - + +#seq_page_cost = 1.0 # measured on an arbitrary scale +#random_page_cost = 4.0 # same scale as above +#cpu_tuple_cost = 0.01 # same scale as above +#cpu_index_tuple_cost = 0.005 # same scale as above +#cpu_operator_cost = 0.0025 # same scale as above +#parallel_tuple_cost = 0.1 # same scale as above +#parallel_setup_cost = 1000.0 # same scale as above +#min_parallel_table_scan_size = 8MB +#min_parallel_index_scan_size = 512kB +#effective_cache_size = 4GB + +# - Genetic Query Optimizer - + +#geqo = on +#geqo_threshold = 12 +#geqo_effort = 5 # range 1-10 +#geqo_pool_size = 0 # selects default based on effort +#geqo_generations = 0 # selects default based on effort +#geqo_selection_bias = 2.0 # range 1.5-2.0 +#geqo_seed = 0.0 # range 0.0-1.0 + +# - Other Planner Options - + +#default_statistics_target = 100 # range 1-10000 +#constraint_exclusion = partition # on, off, or partition +#cursor_tuple_fraction = 0.1 # range 0.0-1.0 +#from_collapse_limit = 8 +#join_collapse_limit = 8 # 1 disables collapsing of explicit + # JOIN clauses +#force_parallel_mode = off + + +#------------------------------------------------------------------------------ +# ERROR REPORTING AND LOGGING +#------------------------------------------------------------------------------ + +# - Where to Log - + +#log_destination = 'stderr' # Valid values are combinations of + # stderr, csvlog, syslog, and eventlog, + # depending on platform. csvlog + # requires logging_collector to be on. + +# This is used when logging to stderr: +#logging_collector = off # Enable capturing of stderr and csvlog + # into log files. Required to be on for + # csvlogs. + # (change requires restart) + +# These are only used if logging_collector is on: +#log_directory = 'log' # directory where log files are written, + # can be absolute or relative to PGDATA +#log_filename = 'postgresql-%Y-%m-%d_%H%M%S.log' # log file name pattern, + # can include strftime() escapes +#log_file_mode = 0600 # creation mode for log files, + # begin with 0 to use octal notation +#log_truncate_on_rotation = off # If on, an existing log file with the + # same name as the new log file will be + # truncated rather than appended to. + # But such truncation only occurs on + # time-driven rotation, not on restarts + # or size-driven rotation. Default is + # off, meaning append to existing files + # in all cases. +#log_rotation_age = 1d # Automatic rotation of logfiles will + # happen after that time. 0 disables. +#log_rotation_size = 10MB # Automatic rotation of logfiles will + # happen after that much log output. + # 0 disables. + +# These are relevant when logging to syslog: +#syslog_facility = 'LOCAL0' +#syslog_ident = 'postgres' +#syslog_sequence_numbers = on +#syslog_split_messages = on + +# This is only relevant when logging to eventlog (win32): +# (change requires restart) +#event_source = 'PostgreSQL' + +# - When to Log - + +log_min_messages = info # values in order of decreasing detail: + # debug5 + # debug4 + # debug3 + # debug2 + # debug1 + # info + # notice + # warning + # error + # log + # fatal + # panic + +log_min_error_statement = info # values in order of decreasing detail: + # debug5 + # debug4 + # debug3 + # debug2 + # debug1 + # info + # notice + # warning + # error + # log + # fatal + # panic (effectively off) + +#log_min_duration_statement = -1 # -1 is disabled, 0 logs all statements + # and their durations, > 0 logs only + # statements running at least this number + # of milliseconds + + +# - What to Log - + +#debug_print_parse = off +#debug_print_rewritten = off +#debug_print_plan = off +#debug_pretty_print = on +#log_checkpoints = off +#log_connections = off +#log_disconnections = off +#log_duration = off +#log_error_verbosity = default # terse, default, or verbose messages +#log_hostname = off +#log_line_prefix = '%m [%p] ' # special values: + # %a = application name + # %u = user name + # %d = database name + # %r = remote host and port + # %h = remote host + # %p = process ID + # %t = timestamp without milliseconds + # %m = timestamp with milliseconds + # %n = timestamp with milliseconds (as a Unix epoch) + # %i = command tag + # %e = SQL state + # %c = session ID + # %l = session line number + # %s = session start timestamp + # %v = virtual transaction ID + # %x = transaction ID (0 if none) + # %q = stop here in non-session + # processes + # %% = '%' + # e.g. '<%u%%%d> ' +#log_lock_waits = off # log lock waits >= deadlock_timeout +#log_statement = 'none' # none, ddl, mod, all +#log_replication_commands = off +#log_temp_files = -1 # log temporary files equal or larger + # than the specified size in kilobytes; + # -1 disables, 0 logs all temp files +log_timezone = 'UTC' + + +# - Process Title - + +#cluster_name = '' # added to process titles if nonempty + # (change requires restart) +#update_process_title = on + + +#------------------------------------------------------------------------------ +# RUNTIME STATISTICS +#------------------------------------------------------------------------------ + +# - Query/Index Statistics Collector - + +#track_activities = on +#track_counts = on +#track_io_timing = off +#track_functions = none # none, pl, all +#track_activity_query_size = 1024 # (change requires restart) +#stats_temp_directory = 'pg_stat_tmp' + + +# - Statistics Monitoring - + +#log_parser_stats = off +#log_planner_stats = off +#log_executor_stats = off +#log_statement_stats = off + + +#------------------------------------------------------------------------------ +# AUTOVACUUM PARAMETERS +#------------------------------------------------------------------------------ + +#autovacuum = on # Enable autovacuum subprocess? 'on' + # requires track_counts to also be on. +#log_autovacuum_min_duration = -1 # -1 disables, 0 logs all actions and + # their durations, > 0 logs only + # actions running at least this number + # of milliseconds. +#autovacuum_max_workers = 3 # max number of autovacuum subprocesses + # (change requires restart) +#autovacuum_naptime = 1min # time between autovacuum runs +#autovacuum_vacuum_threshold = 50 # min number of row updates before + # vacuum +#autovacuum_analyze_threshold = 50 # min number of row updates before + # analyze +#autovacuum_vacuum_scale_factor = 0.2 # fraction of table size before vacuum +#autovacuum_analyze_scale_factor = 0.1 # fraction of table size before analyze +#autovacuum_freeze_max_age = 200000000 # maximum XID age before forced vacuum + # (change requires restart) +#autovacuum_multixact_freeze_max_age = 400000000 # maximum multixact age + # before forced vacuum + # (change requires restart) +#autovacuum_vacuum_cost_delay = 20ms # default vacuum cost delay for + # autovacuum, in milliseconds; + # -1 means use vacuum_cost_delay +#autovacuum_vacuum_cost_limit = -1 # default vacuum cost limit for + # autovacuum, -1 means use + # vacuum_cost_limit + + +#------------------------------------------------------------------------------ +# CLIENT CONNECTION DEFAULTS +#------------------------------------------------------------------------------ + +# - Statement Behavior - + +#client_min_messages = notice # values in order of decreasing detail: + # debug5 + # debug4 + # debug3 + # debug2 + # debug1 + # log + # notice + # warning + # error +#search_path = '"$user", public' # schema names +#default_tablespace = '' # a tablespace name, '' uses the default +#temp_tablespaces = '' # a list of tablespace names, '' uses + # only default tablespace +#check_function_bodies = on +#default_transaction_isolation = 'read committed' +#default_transaction_read_only = off +#default_transaction_deferrable = off +#session_replication_role = 'origin' +statement_timeout = 10000 # in milliseconds, 0 is disabled +lock_timeout = 10000 # in milliseconds, 0 is disabled +#idle_in_transaction_session_timeout = 0 # in milliseconds, 0 is disabled +#vacuum_freeze_min_age = 50000000 +#vacuum_freeze_table_age = 150000000 +#vacuum_multixact_freeze_min_age = 5000000 +#vacuum_multixact_freeze_table_age = 150000000 +#bytea_output = 'hex' # hex, escape +#xmlbinary = 'base64' +#xmloption = 'content' +#gin_fuzzy_search_limit = 0 +#gin_pending_list_limit = 4MB + +# - Locale and Formatting - + +datestyle = 'iso, mdy' +#intervalstyle = 'postgres' +timezone = 'UTC' +#timezone_abbreviations = 'Default' # Select the set of available time zone + # abbreviations. Currently, there are + # Default + # Australia (historical usage) + # India + # You can create your own file in + # share/timezonesets/. +#extra_float_digits = 0 # min -15, max 3 +#client_encoding = sql_ascii # actually, defaults to database + # encoding + +# These settings are initialized by initdb, but they can be changed. +lc_messages = 'en_US.UTF-8' # locale for system error message + # strings +lc_monetary = 'en_US.UTF-8' # locale for monetary formatting +lc_numeric = 'en_US.UTF-8' # locale for number formatting +lc_time = 'en_US.UTF-8' # locale for time formatting + +# default configuration for text search +default_text_search_config = 'pg_catalog.english' + +# - Other Defaults - + +#dynamic_library_path = '$libdir' +#local_preload_libraries = '' +#session_preload_libraries = '' + + +#------------------------------------------------------------------------------ +# LOCK MANAGEMENT +#------------------------------------------------------------------------------ + +#deadlock_timeout = 1s +#max_locks_per_transaction = 64 # min 10 + # (change requires restart) +#max_pred_locks_per_transaction = 64 # min 10 + # (change requires restart) +#max_pred_locks_per_relation = -2 # negative values mean + # (max_pred_locks_per_transaction + # / -max_pred_locks_per_relation) - 1 +#max_pred_locks_per_page = 2 # min 0 + + +#------------------------------------------------------------------------------ +# VERSION/PLATFORM COMPATIBILITY +#------------------------------------------------------------------------------ + +# - Previous PostgreSQL Versions - + +#array_nulls = on +#backslash_quote = safe_encoding # on, off, or safe_encoding +#default_with_oids = off +#escape_string_warning = on +#lo_compat_privileges = off +#operator_precedence_warning = off +#quote_all_identifiers = off +#standard_conforming_strings = on +#synchronize_seqscans = on + +# - Other Platforms and Clients - + +#transform_null_equals = off + + +#------------------------------------------------------------------------------ +# ERROR HANDLING +#------------------------------------------------------------------------------ + +#exit_on_error = off # terminate session on any error? +#restart_after_crash = on # reinitialize after backend crash? +#data_sync_retry = off # retry or panic on failure to fsync + # data? + # (change requires restart) + + +#------------------------------------------------------------------------------ +# CONFIG FILE INCLUDES +#------------------------------------------------------------------------------ + +# These options allow settings to be loaded from files other than the +# default postgresql.conf. Note that these are directives, not variable +# assignments, so they can usefully be given more than once. + +#include_dir = '...' # include files ending in '.conf' from + # a directory, e.g., 'conf.d' +#include_if_exists = '...' # include file only if it exists +#include = '...' # include file + + +#------------------------------------------------------------------------------ +# CUSTOMIZED OPTIONS +#------------------------------------------------------------------------------ + +# Add settings for extensions here From 4247c449f57c5b24c04545c2352fcfce4623282c Mon Sep 17 00:00:00 2001 From: Benjamin Fry Date: Fri, 27 Dec 2019 15:48:06 -0800 Subject: [PATCH 13/57] add extra debug info to pg --- Makefile.toml | 25 +++++++++++++++---------- integration-tests/Makefile.toml | 22 ++++++++++++++++++++++ integration-tests/postgresql.conf | 6 +++--- 3 files changed, 40 insertions(+), 13 deletions(-) create mode 100644 integration-tests/Makefile.toml diff --git a/Makefile.toml b/Makefile.toml index e029432e..806d1144 100644 --- a/Makefile.toml +++ b/Makefile.toml @@ -131,7 +131,7 @@ args = ["install", "cargo-with", "--git=https://github.com/bluejekyll/cargo-with [tasks.pg-init-db-dir] description = "Creates the test DB used by the integration tests" workspace = false -condition = { files_not_exist = ["${PG_DB_DIR}"] } +condition = { files_not_exist = ["${PG_DB_DIR}/pg_wal"] } dependencies = ["install-postgres"] script_runner = "@shell" script = [ @@ -139,6 +139,7 @@ script = [ set -e echo "Intializing postgres db ${PG_DB_DIR:?}" +rm -rf ${PG_DB_DIR:?} mkdir -p ${PG_DB_DIR:?} ${PG_BIN_DIR}/pg_ctl init -D ${PG_DB_DIR:?} -l ${PG_LOGPATH} ''' @@ -264,6 +265,16 @@ rm -rf ${TARGET_DIR:?}/debug ''' ] +[tasks.clean-db] +description = "Removes the DB directory" +workspace = false +script_runner = "@shell" +script = [ +''' +rm -rf ${PG_DB_DIR:?} +''' +] + [tasks.clean-all] description = "Remove only the current workspace member" workspace = false @@ -297,16 +308,10 @@ dependencies = ["install-postgres"] command = "cargo" args = ["build", "--lib", "--bins", "@@remove-empty(FEATURES)"] -[tasks.test-inner] -description = "Run tests on all the crates, without restarting PG" -dependencies = ["pg-create-db"] -env = { POSTGRES_PORT = "${PG_PORT}" } -command = "cargo" -args = ["test", "--jobs=1", "--lib", "--examples", "--tests", "--bins", "@@remove-empty(FEATURES)"] - [tasks.test] -description = "Run tests on all the crates, restarts and stops PG" -dependencies = ["pg-stop", "pg-start", "test-inner", "pg-stop"] +description = "Run tests on all the crates" +command = "cargo" +args = ["test", "--lib", "--examples", "--tests", "--bins", "@@remove-empty(FEATURES)"] [tasks.clippy] description = "Run the clippy linter on all crates" diff --git a/integration-tests/Makefile.toml b/integration-tests/Makefile.toml new file mode 100644 index 00000000..f189a1eb --- /dev/null +++ b/integration-tests/Makefile.toml @@ -0,0 +1,22 @@ +[config] +skip_core_tasks = true + +[config.modify_core_tasks] +# if true, all core tasks are set to private (default false) +private = true + +## Feature profiles +[env] +CARGO_MAKE_EXTEND_WORKSPACE_MAKEFILE = "true" + +[tasks.test-inner] +description = "Run tests on all the crates, without restarting PG" +dependencies = ["pg-create-db"] +env = { POSTGRES_PORT = "${PG_PORT}" } +command = "cargo" +args = ["test", "--jobs=1", "--lib", "--examples", "--tests", "--bins", "@@remove-empty(FEATURES)"] + +[tasks.test] +description = "Run tests on all the crates, restarts and stops PG" +env = { POSTGRES_PORT = "${PG_PORT}" } +dependencies = ["pg-stop", "pg-start", "test-inner", "pg-stop"] diff --git a/integration-tests/postgresql.conf b/integration-tests/postgresql.conf index bc4da121..02738cd8 100644 --- a/integration-tests/postgresql.conf +++ b/integration-tests/postgresql.conf @@ -385,7 +385,7 @@ dynamic_shared_memory_type = posix # the default is the first option # - When to Log - -log_min_messages = info # values in order of decreasing detail: +log_min_messages = debug2 # values in order of decreasing detail: # debug5 # debug4 # debug3 @@ -399,7 +399,7 @@ log_min_messages = info # values in order of decreasing detail: # fatal # panic -log_min_error_statement = info # values in order of decreasing detail: +log_min_error_statement = debug2 # values in order of decreasing detail: # debug5 # debug4 # debug3 @@ -413,7 +413,7 @@ log_min_error_statement = info # values in order of decreasing detail: # fatal # panic (effectively off) -#log_min_duration_statement = -1 # -1 is disabled, 0 logs all statements +log_min_duration_statement = 0 # -1 is disabled, 0 logs all statements # and their durations, > 0 logs only # statements running at least this number # of milliseconds From b6db8806d792849a1a1fbb599f998af8035718a5 Mon Sep 17 00:00:00 2001 From: Benjamin Fry Date: Fri, 27 Dec 2019 16:43:46 -0800 Subject: [PATCH 14/57] ensure all tests are single threaded against PG --- integration-tests/Makefile.toml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/integration-tests/Makefile.toml b/integration-tests/Makefile.toml index f189a1eb..5b00d8c4 100644 --- a/integration-tests/Makefile.toml +++ b/integration-tests/Makefile.toml @@ -14,9 +14,8 @@ description = "Run tests on all the crates, without restarting PG" dependencies = ["pg-create-db"] env = { POSTGRES_PORT = "${PG_PORT}" } command = "cargo" -args = ["test", "--jobs=1", "--lib", "--examples", "--tests", "--bins", "@@remove-empty(FEATURES)"] +args = ["test", "--jobs=1", "--lib", "--examples", "--tests", "--bins", "@@remove-empty(FEATURES)", "--", "--test-threads=1"] [tasks.test] description = "Run tests on all the crates, restarts and stops PG" -env = { POSTGRES_PORT = "${PG_PORT}" } dependencies = ["pg-stop", "pg-start", "test-inner", "pg-stop"] From eef7e02a6c1e4d6ea3875c9ca2dc8a1748cd13ce Mon Sep 17 00:00:00 2001 From: Benjamin Fry Date: Fri, 27 Dec 2019 19:49:23 -0800 Subject: [PATCH 15/57] only run the adding_tests, narrowing down the issue on linux --- integration-tests/tests/fdw.rs | 1 + integration-tests/tests/logging.rs | 2 ++ integration-tests/tests/memory_context.rs | 2 ++ integration-tests/tests/nullable_tests.rs | 2 ++ integration-tests/tests/panicking_tests.rs | 2 ++ integration-tests/tests/strings.rs | 2 ++ 6 files changed, 11 insertions(+) diff --git a/integration-tests/tests/fdw.rs b/integration-tests/tests/fdw.rs index 4b82fe85..09ec35b7 100644 --- a/integration-tests/tests/fdw.rs +++ b/integration-tests/tests/fdw.rs @@ -4,6 +4,7 @@ // http://apache.org/licenses/LICENSE-2.0> or the MIT license , at your option. This file may not be // copied, modified, or distributed except according to those terms. +#![cfg(off)] extern crate integration_tests; diff --git a/integration-tests/tests/logging.rs b/integration-tests/tests/logging.rs index 0675df82..1fde2704 100644 --- a/integration-tests/tests/logging.rs +++ b/integration-tests/tests/logging.rs @@ -1,3 +1,5 @@ +#![cfg(off)] + extern crate integration_tests; use core::mem; diff --git a/integration-tests/tests/memory_context.rs b/integration-tests/tests/memory_context.rs index 79bc0a61..ea476a18 100644 --- a/integration-tests/tests/memory_context.rs +++ b/integration-tests/tests/memory_context.rs @@ -1,3 +1,5 @@ +#![cfg(off)] + extern crate integration_tests; use integration_tests::*; diff --git a/integration-tests/tests/nullable_tests.rs b/integration-tests/tests/nullable_tests.rs index 8c4f5143..058b7c0c 100644 --- a/integration-tests/tests/nullable_tests.rs +++ b/integration-tests/tests/nullable_tests.rs @@ -1,3 +1,5 @@ +#![cfg(off)] + extern crate integration_tests; use integration_tests::*; diff --git a/integration-tests/tests/panicking_tests.rs b/integration-tests/tests/panicking_tests.rs index 4aaca677..c4870a0f 100644 --- a/integration-tests/tests/panicking_tests.rs +++ b/integration-tests/tests/panicking_tests.rs @@ -1,3 +1,5 @@ +#![cfg(off)] + extern crate integration_tests; use integration_tests::*; diff --git a/integration-tests/tests/strings.rs b/integration-tests/tests/strings.rs index 2fd0e6ea..0c234b60 100644 --- a/integration-tests/tests/strings.rs +++ b/integration-tests/tests/strings.rs @@ -1,3 +1,5 @@ +#![cfg(off)] + extern crate integration_tests; use integration_tests::*; From a70ffd9313fbbf0e16e3a429bf42f69a91dce401 Mon Sep 17 00:00:00 2001 From: Benjamin Fry Date: Sat, 28 Dec 2019 08:44:08 -0800 Subject: [PATCH 16/57] reenable logging tests --- integration-tests/tests/logging.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/integration-tests/tests/logging.rs b/integration-tests/tests/logging.rs index 1fde2704..0675df82 100644 --- a/integration-tests/tests/logging.rs +++ b/integration-tests/tests/logging.rs @@ -1,5 +1,3 @@ -#![cfg(off)] - extern crate integration_tests; use core::mem; From 7899ef6ede5c77619851c3ca5791e9b54fc4dac7 Mon Sep 17 00:00:00 2001 From: Benjamin Fry Date: Sat, 28 Dec 2019 09:10:23 -0800 Subject: [PATCH 17/57] double check that logging is the issue --- integration-tests/tests/logging.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/integration-tests/tests/logging.rs b/integration-tests/tests/logging.rs index 0675df82..1fde2704 100644 --- a/integration-tests/tests/logging.rs +++ b/integration-tests/tests/logging.rs @@ -1,3 +1,5 @@ +#![cfg(off)] + extern crate integration_tests; use core::mem; From 7a210802011d01683cc0d69497aee574ecda5983 Mon Sep 17 00:00:00 2001 From: Benjamin Fry Date: Sat, 28 Dec 2019 09:49:30 -0800 Subject: [PATCH 18/57] stop copying file to tmp --- integration-tests/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/integration-tests/src/lib.rs b/integration-tests/src/lib.rs index af8cd1f1..f20f50a9 100644 --- a/integration-tests/src/lib.rs +++ b/integration-tests/src/lib.rs @@ -194,8 +194,8 @@ pub fn test_in_db(lib_name: &str, test: F) { let lib_path = build_lib(lib_name).expect("failed to build extension"); assert!(lib_path.exists()); - let tmpdir = tempfile::tempdir().expect("failed to make tempdir"); - let lib_path = copy_to_tempdir(tmpdir.path(), lib_path); + //let tmpdir = tempfile::tempdir().expect("failed to make tempdir"); + //let lib_path = copy_to_tempdir(tmpdir.path(), lib_path); println!("creating statements with bin: {}", bin_path.display()); run_create_stmts(&bin_path, &lib_path); From fc920970ee4d0e78f13d3474e6b56ef7a852dc4c Mon Sep 17 00:00:00 2001 From: Benjamin Fry Date: Sat, 28 Dec 2019 09:56:39 -0800 Subject: [PATCH 19/57] remove cargo cache from CI, not working --- .github/workflows/test.yml | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 657bfea8..6d51c51b 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -24,15 +24,6 @@ jobs: steps: - uses: actions/checkout@v1 - - name: cargo cache - uses: actions/cache@v1 - with: - path: ~/.cargo - key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.toml') }}-${{ hashFiles('**/Cargo.lock') }} - restore-keys: | - ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.toml') }} - ${{ runner.os }}-cargo - - uses: actions-rs/toolchain@v1 with: profile: minimal @@ -68,15 +59,6 @@ jobs: steps: - uses: actions/checkout@v1 - - name: cargo cache - uses: actions/cache@v1 - with: - path: ~/.cargo - key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.toml') }}-${{ hashFiles('**/Cargo.lock') }} - restore-keys: | - ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.toml') }} - ${{ runner.os }}-cargo - - uses: actions-rs/toolchain@v1 with: profile: minimal From 54e096f6b1083330ed3c7f712084234347867b54 Mon Sep 17 00:00:00 2001 From: Benjamin Fry Date: Sat, 28 Dec 2019 10:09:39 -0800 Subject: [PATCH 20/57] see if panicking tests also cause issues --- integration-tests/tests/panicking_tests.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/integration-tests/tests/panicking_tests.rs b/integration-tests/tests/panicking_tests.rs index c4870a0f..4aaca677 100644 --- a/integration-tests/tests/panicking_tests.rs +++ b/integration-tests/tests/panicking_tests.rs @@ -1,5 +1,3 @@ -#![cfg(off)] - extern crate integration_tests; use integration_tests::*; From 60ba457f063172bf4407af293e84c7eef66928a9 Mon Sep 17 00:00:00 2001 From: Benjamin Fry Date: Sat, 28 Dec 2019 10:20:02 -0800 Subject: [PATCH 21/57] retest with logging enabled --- integration-tests/tests/logging.rs | 2 -- integration-tests/tests/panicking_tests.rs | 2 ++ 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/integration-tests/tests/logging.rs b/integration-tests/tests/logging.rs index 1fde2704..0675df82 100644 --- a/integration-tests/tests/logging.rs +++ b/integration-tests/tests/logging.rs @@ -1,5 +1,3 @@ -#![cfg(off)] - extern crate integration_tests; use core::mem; diff --git a/integration-tests/tests/panicking_tests.rs b/integration-tests/tests/panicking_tests.rs index 4aaca677..c4870a0f 100644 --- a/integration-tests/tests/panicking_tests.rs +++ b/integration-tests/tests/panicking_tests.rs @@ -1,3 +1,5 @@ +#![cfg(off)] + extern crate integration_tests; use integration_tests::*; From 26fc01df142a3e5f27751e05dcda574e0527e728 Mon Sep 17 00:00:00 2001 From: Benjamin Fry Date: Sat, 28 Dec 2019 15:52:05 -0800 Subject: [PATCH 22/57] add back all tests --- integration-tests/tests/fdw.rs | 2 -- integration-tests/tests/memory_context.rs | 2 -- integration-tests/tests/nullable_tests.rs | 2 -- integration-tests/tests/panicking_tests.rs | 2 -- integration-tests/tests/strings.rs | 2 -- 5 files changed, 10 deletions(-) diff --git a/integration-tests/tests/fdw.rs b/integration-tests/tests/fdw.rs index 09ec35b7..ad44c1fb 100644 --- a/integration-tests/tests/fdw.rs +++ b/integration-tests/tests/fdw.rs @@ -4,8 +4,6 @@ // http://apache.org/licenses/LICENSE-2.0> or the MIT license , at your option. This file may not be // copied, modified, or distributed except according to those terms. -#![cfg(off)] - extern crate integration_tests; use integration_tests::*; diff --git a/integration-tests/tests/memory_context.rs b/integration-tests/tests/memory_context.rs index ea476a18..79bc0a61 100644 --- a/integration-tests/tests/memory_context.rs +++ b/integration-tests/tests/memory_context.rs @@ -1,5 +1,3 @@ -#![cfg(off)] - extern crate integration_tests; use integration_tests::*; diff --git a/integration-tests/tests/nullable_tests.rs b/integration-tests/tests/nullable_tests.rs index 058b7c0c..8c4f5143 100644 --- a/integration-tests/tests/nullable_tests.rs +++ b/integration-tests/tests/nullable_tests.rs @@ -1,5 +1,3 @@ -#![cfg(off)] - extern crate integration_tests; use integration_tests::*; diff --git a/integration-tests/tests/panicking_tests.rs b/integration-tests/tests/panicking_tests.rs index c4870a0f..4aaca677 100644 --- a/integration-tests/tests/panicking_tests.rs +++ b/integration-tests/tests/panicking_tests.rs @@ -1,5 +1,3 @@ -#![cfg(off)] - extern crate integration_tests; use integration_tests::*; diff --git a/integration-tests/tests/strings.rs b/integration-tests/tests/strings.rs index 0c234b60..2fd0e6ea 100644 --- a/integration-tests/tests/strings.rs +++ b/integration-tests/tests/strings.rs @@ -1,5 +1,3 @@ -#![cfg(off)] - extern crate integration_tests; use integration_tests::*; From 86b15370a248c078c6d00b5507e0d2f19fb2a572 Mon Sep 17 00:00:00 2001 From: Benjamin Fry Date: Sat, 28 Dec 2019 16:47:39 -0800 Subject: [PATCH 23/57] remove parallel restrictions --- Makefile.toml | 8 ++++---- integration-tests/Makefile.toml | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Makefile.toml b/Makefile.toml index 806d1144..609023f2 100644 --- a/Makefile.toml +++ b/Makefile.toml @@ -300,24 +300,24 @@ args = ["fmt", "--", "--check"] description = "Run a quick check on all the crates" dependencies = ["install-postgres"] command = "cargo" -args = ["check", "--lib", "--examples", "--tests", "--bins", "@@remove-empty(FEATURES)"] +args = ["check", "--all-targets", "@@remove-empty(FEATURES)"] [tasks.build] description = "Build all the crates" dependencies = ["install-postgres"] command = "cargo" -args = ["build", "--lib", "--bins", "@@remove-empty(FEATURES)"] +args = ["build", "--all-targets", "@@remove-empty(FEATURES)"] [tasks.test] description = "Run tests on all the crates" command = "cargo" -args = ["test", "--lib", "--examples", "--tests", "--bins", "@@remove-empty(FEATURES)"] +args = ["test", "--all-targets", "@@remove-empty(FEATURES)"] [tasks.clippy] description = "Run the clippy linter on all crates" dependencies = ["clean", "install-postgres"] command = "cargo" -args = ["clippy", "--lib", "--examples", "--tests", "--bins", "${ALL_FEATURES}", "--", "-D", "warnings"] +args = ["clippy", "--all-targets", "${ALL_FEATURES}", "--", "-D", "warnings"] [tasks.build-bench] description = "Check that all benchmarks compile" diff --git a/integration-tests/Makefile.toml b/integration-tests/Makefile.toml index 5b00d8c4..bb8c12ab 100644 --- a/integration-tests/Makefile.toml +++ b/integration-tests/Makefile.toml @@ -14,7 +14,7 @@ description = "Run tests on all the crates, without restarting PG" dependencies = ["pg-create-db"] env = { POSTGRES_PORT = "${PG_PORT}" } command = "cargo" -args = ["test", "--jobs=1", "--lib", "--examples", "--tests", "--bins", "@@remove-empty(FEATURES)", "--", "--test-threads=1"] +args = ["test", "--all-targets", "@@remove-empty(FEATURES)"] [tasks.test] description = "Run tests on all the crates, restarts and stops PG" From f661b0dad465b6964c63f5895bbc96e9ab1b2cb0 Mon Sep 17 00:00:00 2001 From: Benjamin Fry Date: Sat, 28 Dec 2019 17:37:08 -0800 Subject: [PATCH 24/57] change clippy to do entire workspace in one go --- Makefile.toml | 9 +++++++-- pg-extend/build.rs | 12 +++++------- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/Makefile.toml b/Makefile.toml index 609023f2..092a1694 100644 --- a/Makefile.toml +++ b/Makefile.toml @@ -42,6 +42,7 @@ POSTGRES_TEST_DB = "pg_extend_rs_test_db" PG_LOGPATH = "${TARGET_DIR}/postgres-${PG_VERSION}.log" PATH = "${PG_BIN_DIR}:${PATH}" +PG_CONFIG = "${PG_BIN_DIR}/pg_config" ## ## Installation tasks @@ -315,9 +316,13 @@ args = ["test", "--all-targets", "@@remove-empty(FEATURES)"] [tasks.clippy] description = "Run the clippy linter on all crates" -dependencies = ["clean", "install-postgres"] +#dependencies = ["clean", "install-postgres"] +dependencies = ["clean-build", "install-postgres"] command = "cargo" -args = ["clippy", "--all-targets", "${ALL_FEATURES}", "--", "-D", "warnings"] +#args = ["clippy", "--all-targets", "${ALL_FEATURES}", "--", "-D", "warnings"] +# FIXME: the cbove command is correct, but seems to hit an issue with clippy and library paths, not clear why... +workspace = false +args = ["clippy", "--all", "--all-targets", "--", "-D", "warnings"] [tasks.build-bench] description = "Check that all benchmarks compile" diff --git a/pg-extend/build.rs b/pg-extend/build.rs index c9ce34f6..710a03a8 100644 --- a/pg-extend/build.rs +++ b/pg-extend/build.rs @@ -16,12 +16,13 @@ use std::process::Command; fn main() { let out_path = PathBuf::from(env::var("OUT_DIR").unwrap()).join("postgres.rs"); + let pg_config = env::var("PG_CONFIG").unwrap_or_else(|_| "pg_config".to_string()); + // Re-run this if wrapper.h changes println!("cargo:rerun-if-changed=wrapper.h"); println!("cargo:rerun-if-changed=pg_majorversion.h"); - println!("cargo:rerun-if-env-changed=PG_INCLUDE_PATH"); - let pg_include = include_dir() + let pg_include = include_dir(&pg_config) .expect("set environment variable PG_INCLUDE_PATH to the Postgres install include dir, e.g. /var/lib/pgsql/include/server"); // these cause duplicate definition problems on linux @@ -154,12 +155,9 @@ fn get_bindings(pg_include: &str) -> bindgen::Builder { bindgen::Builder::default().clang_arg(format!("-I{}", pg_include)) } -fn include_dir() -> Result { +fn include_dir(pg_config: &str) -> Result { env::var("PG_INCLUDE_PATH").or_else(|err| { - match Command::new("pg_config") - .arg("--includedir-server") - .output() - { + match Command::new(pg_config).arg("--includedir-server").output() { Ok(out) => Ok(String::from_utf8(out.stdout).unwrap().trim().to_string()), Err(..) => Err(err), } From 1c635d4d1802ff3e513ac1562f550067c7fe3680 Mon Sep 17 00:00:00 2001 From: Benjamin Fry Date: Sat, 28 Dec 2019 18:07:55 -0800 Subject: [PATCH 25/57] remove sleeps --- integration-tests/src/lib.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/integration-tests/src/lib.rs b/integration-tests/src/lib.rs index f20f50a9..30f0c75c 100644 --- a/integration-tests/src/lib.rs +++ b/integration-tests/src/lib.rs @@ -147,8 +147,6 @@ pub fn run_create_stmts(bin_path: &PathBuf, lib_path: &PathBuf) { if let Err(err) = result { error = Some(err); } else { - // FIXME: this sleep helps all the tests pass, PG is finding files not ready?? - std::thread::sleep(std::time::Duration::from_millis(100)); return; } } @@ -178,16 +176,10 @@ pub fn copy_to_tempdir(path: &Path, lib_path: PathBuf) -> PathBuf { }) .unwrap(); - // FIXME: this sleep helps all the tests pass, PG is finding files not ready?? - std::thread::sleep(std::time::Duration::from_millis(100)); - tmplib } pub fn test_in_db(lib_name: &str, test: F) { - // FIXME: this sleep helps all the tests pass, PG is finding files not ready?? - std::thread::sleep(std::time::Duration::from_millis(100)); - println!("test_in_db: {}", lib_name); let bin_path = build_bin(lib_name).expect("failed to build stmt binary"); assert!(bin_path.exists()); From 776921e266a35e4a8213cd69038c837ea84c422d Mon Sep 17 00:00:00 2001 From: Marc-Antoine Enriquez Date: Sun, 29 Dec 2019 03:45:29 +0100 Subject: [PATCH 26/57] Add array support --- pg-extend/build.rs | 2 ++ pg-extend/src/pg_datum.rs | 54 +++++++++++++++++++++++++++++++++++++++ pg-extend/src/pg_type.rs | 9 +++++-- 3 files changed, 63 insertions(+), 2 deletions(-) diff --git a/pg-extend/build.rs b/pg-extend/build.rs index 710a03a8..238e6c15 100644 --- a/pg-extend/build.rs +++ b/pg-extend/build.rs @@ -90,6 +90,7 @@ fn get_bindings(pg_include: &str) -> bindgen::Builder { .whitelist_function("errfinish") .whitelist_function("pfree") .whitelist_function("list_.*") + .whitelist_function("palloc") // Whitelist all PG-related types .whitelist_type("PG.*") // Whitelist used types @@ -97,6 +98,7 @@ fn get_bindings(pg_include: &str) -> bindgen::Builder { .whitelist_type("text") .whitelist_type("varattrib_1b") .whitelist_type("varattrib_4b") + .whitelist_type(".*Array.*") // Whitelist PG-related values .whitelist_var("PG.*") // Whitelist log-level values diff --git a/pg-extend/src/pg_datum.rs b/pg-extend/src/pg_datum.rs index 87ecad84..e842d38c 100644 --- a/pg-extend/src/pg_datum.rs +++ b/pg-extend/src/pg_datum.rs @@ -370,6 +370,60 @@ where } } +impl<'s, T> TryFromPgDatum<'s> for &[T] +where + T: 's + TryFromPgDatum<'s>, +{ + fn try_from<'mc>( + _memory_context: &'mc PgAllocator, + datum: PgDatum<'mc>, + ) -> Result + where + Self: 's, + 'mc: 's, + { + if let Some(datum) = datum.0 { + unsafe { + let datum = datum as *mut pg_sys::varlena; + if datum.is_null() { + return Err("datum was NULL"); + } + + let arr_type = pg_sys::pg_detoast_datum(datum) as *mut pg_sys::ArrayType; + + if (*arr_type).ndim > 1 { + return Err("argument must be empty or one-dimensional array"); + } + + let nulls = 0 as *mut *mut i8; + let elements = 0 as *mut *mut Datum; + let nelems = 0 as *mut i32; + + pg_sys::deconstruct_array(arr_type, (*arr_type).elemtype, + -1, pgbool!(false), b'i' as i8, + elements, nulls, nelems, + ); + + Ok(std::slice::from_raw_parts(elements as *const T, *nelems as usize)) + } + } else { + Err("datum was NULL") + } + } +} + +impl<'mc, 's, T> From<&[T]> for PgDatum<'mc> +where + 'mc: 's, + T: 's, + PgDatum<'mc>: From, +{ + fn from(_value: &[T]) -> Self { + // TODO + PgDatum(None, PhantomData) + } +} + impl From<()> for PgDatum<'static> { fn from(_value: ()) -> Self { PgDatum(None, PhantomData) diff --git a/pg-extend/src/pg_type.rs b/pg-extend/src/pg_type.rs index 4c665ef8..83e39fc9 100644 --- a/pg-extend/src/pg_type.rs +++ b/pg-extend/src/pg_type.rs @@ -246,8 +246,13 @@ impl PgTypeInfo for Text<'_> { fn pg_type() -> PgType { PgType::Text } +} - fn is_option() -> bool { - false +impl PgTypeInfo for &[T] +where + T: PgTypeInfo, +{ + fn pg_type() -> PgType { + T::pg_type() } } From fb5bb60670d076868166d7b43c9ca14aa0aaf53a Mon Sep 17 00:00:00 2001 From: Marc-Antoine Enriquez Date: Sun, 29 Dec 2019 03:45:51 +0100 Subject: [PATCH 27/57] Example for array sum --- examples/adding/src/bin.rs | 3 ++- examples/adding/src/lib.rs | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/examples/adding/src/bin.rs b/examples/adding/src/bin.rs index 1f24bc6e..21894663 100644 --- a/examples/adding/src/bin.rs +++ b/examples/adding/src/bin.rs @@ -6,5 +6,6 @@ pg_create_stmt_bin!( add_one_pg_create_stmt, add_big_one_pg_create_stmt, add_small_one_pg_create_stmt, - add_together_pg_create_stmt + add_together_pg_create_stmt, + sum_array_pg_create_stmt ); diff --git a/examples/adding/src/lib.rs b/examples/adding/src/lib.rs index 55c4f621..658f4452 100644 --- a/examples/adding/src/lib.rs +++ b/examples/adding/src/lib.rs @@ -38,6 +38,11 @@ fn add_together(v1: i64, v2: i32, v3: i16) -> i64 { (v1 + i64::from(v2) + i64::from(v3)) } +#[pg_extern] +fn sum_array(arr: &[i32]) -> i32 { + arr.iter().sum() +} + #[cfg(test)] mod tests { use super::*; From 8501f81d8e7874a93fe33777dd71bc9c1fc97c27 Mon Sep 17 00:00:00 2001 From: Marc-Antoine Enriquez Date: Sun, 29 Dec 2019 05:29:19 +0100 Subject: [PATCH 28/57] Fix array support --- Cargo.lock | 7 +++++++ pg-extend/Cargo.toml | 1 + pg-extend/build.rs | 3 +++ pg-extend/src/pg_datum.rs | 20 ++++++++++++++------ 4 files changed, 25 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8940fa91..89f510cf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1097,6 +1097,7 @@ dependencies = [ "bindgen 0.52.0 (registry+https://github.com/rust-lang/crates.io-index)", "cfg-if 0.1.10 (registry+https://github.com/rust-lang/crates.io-index)", "clang 0.23.0 (registry+https://github.com/rust-lang/crates.io-index)", + "slice-cast 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] @@ -1441,6 +1442,11 @@ name = "slab" version = "0.4.2" source = "registry+https://github.com/rust-lang/crates.io-index" +[[package]] +name = "slice-cast" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" + [[package]] name = "smallvec" version = "1.1.0" @@ -1921,6 +1927,7 @@ dependencies = [ "checksum siphasher 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)" = "83da420ee8d1a89e640d0948c646c1c088758d3a3c538f943bfa97bdac17929d" "checksum sized-chunks 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)" = "f01db57d7ee89c8e053245deb77040a6cc8508311f381c88749c33d4b9b78785" "checksum slab 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)" = "c111b5bd5695e56cffe5129854aa230b39c93a305372fdbb2668ca2394eea9f8" +"checksum slice-cast 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "ce1f91576fca3c2380ff517b741a0e637c83ae9448283b4db029ffec201be988" "checksum smallvec 1.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "44e59e0c9fa00817912ae6e4e6e3c4fe04455e75699d06eedc7d85917ed8e8f4" "checksum socket2 0.3.11 (registry+https://github.com/rust-lang/crates.io-index)" = "e8b74de517221a2cb01a53349cf54182acdc31a074727d3079068448c0676d85" "checksum stringprep 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "8ee348cb74b87454fff4b551cbf727025810a004f88aeacae7f85b87f4e9a1c1" diff --git a/pg-extend/Cargo.toml b/pg-extend/Cargo.toml index e1ea6e81..852a2475 100644 --- a/pg-extend/Cargo.toml +++ b/pg-extend/Cargo.toml @@ -33,6 +33,7 @@ postgres-12 = [] [dependencies] cfg-if = "0.1.10" +slice-cast = "0.1" [build-dependencies] bindgen = "0.52" diff --git a/pg-extend/build.rs b/pg-extend/build.rs index 238e6c15..58618bb0 100644 --- a/pg-extend/build.rs +++ b/pg-extend/build.rs @@ -91,6 +91,8 @@ fn get_bindings(pg_include: &str) -> bindgen::Builder { .whitelist_function("pfree") .whitelist_function("list_.*") .whitelist_function("palloc") + .whitelist_function(".*array.*") + .whitelist_function("get_typlenbyvalalign") // Whitelist all PG-related types .whitelist_type("PG.*") // Whitelist used types @@ -99,6 +101,7 @@ fn get_bindings(pg_include: &str) -> bindgen::Builder { .whitelist_type("varattrib_1b") .whitelist_type("varattrib_4b") .whitelist_type(".*Array.*") + .whitelist_type("bool_") // Whitelist PG-related values .whitelist_var("PG.*") // Whitelist log-level values diff --git a/pg-extend/src/pg_datum.rs b/pg-extend/src/pg_datum.rs index e842d38c..55639604 100644 --- a/pg-extend/src/pg_datum.rs +++ b/pg-extend/src/pg_datum.rs @@ -395,16 +395,24 @@ where return Err("argument must be empty or one-dimensional array"); } - let nulls = 0 as *mut *mut i8; - let elements = 0 as *mut *mut Datum; - let nelems = 0 as *mut i32; + let mut elmlen: pg_sys::int16 = 0; + let mut elmbyval: pg_sys::bool_ = 0; + let mut elmalign: ::std::os::raw::c_char = 0; + + pg_sys::get_typlenbyvalalign((*arr_type).elemtype, &mut elmlen, &mut elmbyval, &mut elmalign); + + let mut nulls = 0 as *mut pg_sys::bool_; + let mut elements = 0 as *mut Datum; + let mut nelems: i32 = 0; pg_sys::deconstruct_array(arr_type, (*arr_type).elemtype, - -1, pgbool!(false), b'i' as i8, - elements, nulls, nelems, + elmlen as i32, elmbyval, elmalign, + &mut elements, &mut nulls, &mut nelems, ); - Ok(std::slice::from_raw_parts(elements as *const T, *nelems as usize)) + let datums = std::slice::from_raw_parts(elements as *const Datum, nelems as usize); + + Ok(slice_cast::cast(datums)) } } else { Err("datum was NULL") From 581de6da73ac204b713a57dec03b1d1c4bf73043 Mon Sep 17 00:00:00 2001 From: Marc-Antoine Enriquez Date: Sun, 29 Dec 2019 15:23:07 +0100 Subject: [PATCH 29/57] Remove dependency on slice_cast crate --- Cargo.lock | 7 ------- pg-extend/Cargo.toml | 1 - pg-extend/src/pg_datum.rs | 11 ++++++++++- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 89f510cf..8940fa91 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1097,7 +1097,6 @@ dependencies = [ "bindgen 0.52.0 (registry+https://github.com/rust-lang/crates.io-index)", "cfg-if 0.1.10 (registry+https://github.com/rust-lang/crates.io-index)", "clang 0.23.0 (registry+https://github.com/rust-lang/crates.io-index)", - "slice-cast 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] @@ -1442,11 +1441,6 @@ name = "slab" version = "0.4.2" source = "registry+https://github.com/rust-lang/crates.io-index" -[[package]] -name = "slice-cast" -version = "0.1.2" -source = "registry+https://github.com/rust-lang/crates.io-index" - [[package]] name = "smallvec" version = "1.1.0" @@ -1927,7 +1921,6 @@ dependencies = [ "checksum siphasher 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)" = "83da420ee8d1a89e640d0948c646c1c088758d3a3c538f943bfa97bdac17929d" "checksum sized-chunks 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)" = "f01db57d7ee89c8e053245deb77040a6cc8508311f381c88749c33d4b9b78785" "checksum slab 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)" = "c111b5bd5695e56cffe5129854aa230b39c93a305372fdbb2668ca2394eea9f8" -"checksum slice-cast 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "ce1f91576fca3c2380ff517b741a0e637c83ae9448283b4db029ffec201be988" "checksum smallvec 1.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "44e59e0c9fa00817912ae6e4e6e3c4fe04455e75699d06eedc7d85917ed8e8f4" "checksum socket2 0.3.11 (registry+https://github.com/rust-lang/crates.io-index)" = "e8b74de517221a2cb01a53349cf54182acdc31a074727d3079068448c0676d85" "checksum stringprep 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "8ee348cb74b87454fff4b551cbf727025810a004f88aeacae7f85b87f4e9a1c1" diff --git a/pg-extend/Cargo.toml b/pg-extend/Cargo.toml index 852a2475..e1ea6e81 100644 --- a/pg-extend/Cargo.toml +++ b/pg-extend/Cargo.toml @@ -33,7 +33,6 @@ postgres-12 = [] [dependencies] cfg-if = "0.1.10" -slice-cast = "0.1" [build-dependencies] bindgen = "0.52" diff --git a/pg-extend/src/pg_datum.rs b/pg-extend/src/pg_datum.rs index 55639604..8a714a4b 100644 --- a/pg-extend/src/pg_datum.rs +++ b/pg-extend/src/pg_datum.rs @@ -412,7 +412,16 @@ where let datums = std::slice::from_raw_parts(elements as *const Datum, nelems as usize); - Ok(slice_cast::cast(datums)) + let mem_size_datums = std::mem::size_of_val(datums); + let datums = if mem_size_datums == 0 { + std::slice::from_raw_parts(datums.as_ptr() as *const T, 0) + } else { + let mem_size_type = std::mem::size_of::(); + assert_eq!(mem_size_datums % mem_size_type, 0); + std::slice::from_raw_parts(datums.as_ptr() as *const T, mem_size_datums / mem_size_type) + }; + + Ok(datums) } } else { Err("datum was NULL") From 2f21d21ef3b3abb5ec643915244a0a7ed9ffe6a3 Mon Sep 17 00:00:00 2001 From: Marc-Antoine Enriquez Date: Sun, 29 Dec 2019 17:59:54 +0100 Subject: [PATCH 30/57] DRY --- examples/adding/src/bin.rs | 6 +- examples/adding/src/lib.rs | 29 +++- pg-extend/src/pg_datum.rs | 267 ++++++++++++++++++------------------- 3 files changed, 165 insertions(+), 137 deletions(-) diff --git a/examples/adding/src/bin.rs b/examples/adding/src/bin.rs index 21894663..2594eb1c 100644 --- a/examples/adding/src/bin.rs +++ b/examples/adding/src/bin.rs @@ -7,5 +7,9 @@ pg_create_stmt_bin!( add_big_one_pg_create_stmt, add_small_one_pg_create_stmt, add_together_pg_create_stmt, - sum_array_pg_create_stmt + sum_array_pg_create_stmt, + sum_small_array_pg_create_stmt, + sum_big_array_pg_create_stmt, + sum_float_array_pg_create_stmt, + sum_double_array_pg_create_stmt ); diff --git a/examples/adding/src/lib.rs b/examples/adding/src/lib.rs index 658f4452..456e25a7 100644 --- a/examples/adding/src/lib.rs +++ b/examples/adding/src/lib.rs @@ -26,23 +26,48 @@ fn add_small_one(value: i16) -> i16 { (value + 1) } -/// Test the i16 value +/// Test the i64 value #[pg_extern] fn add_big_one(value: i64) -> i64 { (value + 1) } -/// Test the i16 value +/// Test all 3 values at a time #[pg_extern] fn add_together(v1: i64, v2: i32, v3: i16) -> i64 { (v1 + i64::from(v2) + i64::from(v3)) } +// Test array of i32 #[pg_extern] fn sum_array(arr: &[i32]) -> i32 { arr.iter().sum() } +// Test array of i16 +#[pg_extern] +fn sum_small_array(arr: &[i16]) -> i16 { + arr.iter().sum() +} + +// Test array of i64 +#[pg_extern] +fn sum_big_array(arr: &[i64]) -> i64 { + arr.iter().sum() +} + +// Test array of f32 +#[pg_extern] +fn sum_float_array(arr: &[f32]) -> f32 { + arr.iter().sum() +} + +// Test array of f32 +#[pg_extern] +fn sum_double_array(arr: &[f64]) -> f64 { + arr.iter().sum() +} + #[cfg(test)] mod tests { use super::*; diff --git a/pg-extend/src/pg_datum.rs b/pg-extend/src/pg_datum.rs index 8a714a4b..9c650065 100644 --- a/pg-extend/src/pg_datum.rs +++ b/pg-extend/src/pg_datum.rs @@ -9,6 +9,7 @@ use std::ffi::{CStr, CString}; use std::marker::PhantomData; +use std::os::raw::c_char; use std::ptr::NonNull; use crate::native::Text; @@ -82,20 +83,38 @@ pub trait TryFromPgDatum<'s>: Sized { ) -> Result where Self: 's, - 'mc: 's; + 'mc: 's, + { + if let Some(datum) = datum.0 { + unsafe { + Self::try_cast(memory_context, datum) + } + } else { + Err("datum was NULL") + } + } + + /// Used for force casting + unsafe fn try_cast<'mc>( + memory_context: &'mc PgAllocator, + datum: Datum, + ) -> Result + where + Self: 's, + 'mc: 's, + ; } impl<'s> TryFromPgDatum<'s> for i16 { - fn try_from<'mc>(_: &'mc PgAllocator, datum: PgDatum<'mc>) -> Result + unsafe fn try_cast<'mc>( + _: &'mc PgAllocator, + datum: Datum, + ) -> Result where Self: 's, 'mc: 's, { - if let Some(datum) = datum.0 { - Ok(datum as i16) - } else { - Err("datum was NULL") - } + Ok(datum as i16) } } @@ -106,16 +125,15 @@ impl From for PgDatum<'_> { } impl<'s> TryFromPgDatum<'s> for f32 { - fn try_from<'mc>(_: &'mc PgAllocator, datum: PgDatum<'mc>) -> Result + unsafe fn try_cast<'mc>( + _: &'mc PgAllocator, + datum: Datum, + ) -> Result where Self: 's, 'mc: 's, { - if let Some(datum) = datum.0 { - Ok(f32::from_bits(datum as u32)) - } else { - Err("datum was NULL") - } + Ok(f32::from_bits(datum as u32)) } } @@ -126,16 +144,15 @@ impl From for PgDatum<'_> { } impl<'s> TryFromPgDatum<'s> for f64 { - fn try_from<'mc>(_: &'mc PgAllocator, datum: PgDatum<'mc>) -> Result + unsafe fn try_cast<'mc>( + _: &'mc PgAllocator, + datum: Datum, + ) -> Result where Self: 's, 'mc: 's, { - if let Some(datum) = datum.0 { - Ok(f64::from_bits(datum as u64)) - } else { - Err("datum was NULL") - } + Ok(f64::from_bits(datum as u64)) } } @@ -146,16 +163,15 @@ impl From for PgDatum<'_> { } impl<'s> TryFromPgDatum<'s> for i32 { - fn try_from<'mc>(_: &'mc PgAllocator, datum: PgDatum<'mc>) -> Result + unsafe fn try_cast<'mc>( + _: &'mc PgAllocator, + datum: Datum, + ) -> Result where Self: 's, 'mc: 's, { - if let Some(datum) = datum.0 { - Ok(datum as i32) - } else { - Err("datum was NULL") - } + Ok(datum as i32) } } @@ -166,20 +182,15 @@ impl From for PgDatum<'_> { } impl<'s> TryFromPgDatum<'s> for i64 { - fn try_from<'mc>(_: &'mc PgAllocator, datum: PgDatum<'mc>) -> Result + unsafe fn try_cast<'mc>( + _: &'mc PgAllocator, + datum: Datum, + ) -> Result where Self: 's, 'mc: 's, { - assert!( - std::mem::size_of::() >= std::mem::size_of::(), - "Datum not large enough for i64 values" - ); - if let Some(datum) = datum.0 { - Ok(datum as i64) - } else { - Err("datum was NULL") - } + Ok(datum as i64) } } @@ -208,13 +219,19 @@ impl<'s> TryFromPgDatum<'s> for String { cstr.into_string() .map_err(|_| "String contained non-utf8 data") } + + unsafe fn try_cast<'mc>(_: &'mc PgAllocator, _: Datum) -> Result + where + Self: 's, + 'mc: 's, + { + unimplemented!("Cast is not implemented for this type"); + } } // FIXME: this lifetime is wrong impl From for PgDatum<'_> { fn from(value: String) -> Self { - use std::os::raw::c_char; - let cstr = CString::new(value).expect("This shouldn't fail"); let ptr: *const c_char = cstr.as_ptr(); @@ -226,37 +243,30 @@ impl From for PgDatum<'_> { #[deprecated(note = "String is not Zero cost, please use the CString variant")] impl<'s> TryFromPgDatum<'s> for CString { - fn try_from<'mc>(_: &'mc PgAllocator, datum: PgDatum<'mc>) -> Result + unsafe fn try_cast<'mc>( + _: &'mc PgAllocator, + datum: Datum, + ) -> Result where Self: 's, 'mc: 's, { - use std::os::raw::c_char; - - if let Some(datum) = datum.0 { - let text_val = datum as *const pg_sys::text; + let text_val = datum as *const pg_sys::text; - unsafe { - crate::guard_pg(|| { - let val: *mut c_char = pg_sys::text_to_cstring(text_val); - let cstr = CStr::from_ptr(val).to_owned(); + crate::guard_pg(|| { + let val: *mut c_char = pg_sys::text_to_cstring(text_val); + let cstr = CStr::from_ptr(val).to_owned(); - pg_sys::pfree(val as *mut _); + pg_sys::pfree(val as *mut _); - Ok(cstr) - }) - } - } else { - Err("datum was NULL") - } + Ok(cstr) + }) } } // FIXME: this lifetime is wrong impl From for PgDatum<'_> { fn from(value: CString) -> Self { - use std::os::raw::c_char; - let ptr: *const c_char = value.as_ptr(); let text = unsafe { crate::guard_pg(|| pg_sys::cstring_to_text(ptr)) }; @@ -265,45 +275,36 @@ impl From for PgDatum<'_> { } impl<'s> TryFromPgDatum<'s> for PgAllocated<'s, CString> { - fn try_from<'mc>( + unsafe fn try_cast<'mc>( memory_context: &'mc PgAllocator, - datum: PgDatum<'mc>, + datum: Datum, ) -> Result where Self: 's, 'mc: 's, { - use std::os::raw::c_char; - - if let Some(datum) = datum.0 { - let text_val = datum as *const pg_sys::text; - - unsafe { - crate::guard_pg(|| { - // from varlena.c - /* - * text_to_cstring - * - * Create a palloc'd, null-terminated C string from a text value. - * - * We support being passed a compressed or toasted text value. - * This is a bit bogus since such values shouldn't really be referred to as - * "text *", but it seems useful for robustness. If we didn't handle that - * case here, we'd need another routine that did, anyway. - */ - let cstr = pg_sys::text_to_cstring(text_val) as *mut c_char; - - // this is dangerous! it's owned by CString, which is why PgAllocated will - // block the dealloc - //let cstr = CString::from_raw(val); - let allocated = PgAllocated::from_raw(memory_context, cstr); - - Ok(allocated) - }) - } - } else { - Err("datum was NULL") - } + let text_val = datum as *const pg_sys::text; + crate::guard_pg(|| { + // from varlena.c + /* + * text_to_cstring + * + * Create a palloc'd, null-terminated C string from a text value. + * + * We support being passed a compressed or toasted text value. + * This is a bit bogus since such values shouldn't really be referred to as + * "text *", but it seems useful for robustness. If we didn't handle that + * case here, we'd need another routine that did, anyway. + */ + let cstr = pg_sys::text_to_cstring(text_val) as *mut c_char; + + // this is dangerous! it's owned by CString, which is why PgAllocated will + // block the dealloc + //let cstr = CString::from_raw(val); + let allocated = PgAllocated::from_raw(memory_context, cstr); + + Ok(allocated) + }) } } @@ -315,21 +316,17 @@ impl<'s> From> for PgDatum<'s> { } impl<'s> TryFromPgDatum<'s> for Text<'s> { - fn try_from<'mc>( + unsafe fn try_cast<'mc>( memory_context: &'mc PgAllocator, - datum: PgDatum<'mc>, + datum: Datum, ) -> Result where Self: 's, 'mc: 's, { - if let Some(datum) = datum.0 { - let text_ptr = datum as *const pg_sys::text; + let text_ptr = datum as *const pg_sys::text; - unsafe { Ok(Text::from_raw(memory_context, text_ptr as *mut _)) } - } else { - Err("datum was NULL") - } + Ok(Text::from_raw(memory_context, text_ptr as *mut _)) } } @@ -354,6 +351,14 @@ where Ok(Some(result?)) } + + unsafe fn try_cast<'mc>(_: &'mc PgAllocator, _: Datum) -> Result + where + Self: 's, + 'mc: 's, + { + unimplemented!("Cast is not implemented for this type"); + } } impl<'mc, 's, T> From> for PgDatum<'mc> @@ -374,58 +379,52 @@ impl<'s, T> TryFromPgDatum<'s> for &[T] where T: 's + TryFromPgDatum<'s>, { - fn try_from<'mc>( - _memory_context: &'mc PgAllocator, - datum: PgDatum<'mc>, + unsafe fn try_cast<'mc>( + _: &'mc PgAllocator, + datum: Datum, ) -> Result where Self: 's, 'mc: 's, { - if let Some(datum) = datum.0 { - unsafe { - let datum = datum as *mut pg_sys::varlena; - if datum.is_null() { - return Err("datum was NULL"); - } - - let arr_type = pg_sys::pg_detoast_datum(datum) as *mut pg_sys::ArrayType; + let datum = datum as *mut pg_sys::varlena; + if datum.is_null() { + return Err("datum was NULL"); + } - if (*arr_type).ndim > 1 { - return Err("argument must be empty or one-dimensional array"); - } + let arr_type = pg_sys::pg_detoast_datum(datum) as *mut pg_sys::ArrayType; - let mut elmlen: pg_sys::int16 = 0; - let mut elmbyval: pg_sys::bool_ = 0; - let mut elmalign: ::std::os::raw::c_char = 0; + if (*arr_type).ndim > 1 { + return Err("argument must be empty or one-dimensional array"); + } - pg_sys::get_typlenbyvalalign((*arr_type).elemtype, &mut elmlen, &mut elmbyval, &mut elmalign); + let mut elmlen: pg_sys::int16 = 0; + let mut elmbyval: pg_sys::bool_ = 0; + let mut elmalign: ::std::os::raw::c_char = 0; - let mut nulls = 0 as *mut pg_sys::bool_; - let mut elements = 0 as *mut Datum; - let mut nelems: i32 = 0; + pg_sys::get_typlenbyvalalign((*arr_type).elemtype, &mut elmlen, &mut elmbyval, &mut elmalign); - pg_sys::deconstruct_array(arr_type, (*arr_type).elemtype, - elmlen as i32, elmbyval, elmalign, - &mut elements, &mut nulls, &mut nelems, - ); + let mut nulls = 0 as *mut pg_sys::bool_; + let mut elements = 0 as *mut Datum; + let mut nelems: i32 = 0; - let datums = std::slice::from_raw_parts(elements as *const Datum, nelems as usize); + pg_sys::deconstruct_array(arr_type, (*arr_type).elemtype, + elmlen as i32, elmbyval, elmalign, + &mut elements, &mut nulls, &mut nelems, + ); - let mem_size_datums = std::mem::size_of_val(datums); - let datums = if mem_size_datums == 0 { - std::slice::from_raw_parts(datums.as_ptr() as *const T, 0) - } else { - let mem_size_type = std::mem::size_of::(); - assert_eq!(mem_size_datums % mem_size_type, 0); - std::slice::from_raw_parts(datums.as_ptr() as *const T, mem_size_datums / mem_size_type) - }; + let datums = std::slice::from_raw_parts(elements as *const Datum, nelems as usize); - Ok(datums) - } + let mem_size_datums = std::mem::size_of_val(datums); + let datums = if mem_size_datums == 0 { + std::slice::from_raw_parts(datums.as_ptr() as *const T, 0) } else { - Err("datum was NULL") - } + let mem_size_type = std::mem::size_of::(); + assert_eq!(mem_size_datums % mem_size_type, 0); + std::slice::from_raw_parts(datums.as_ptr() as *const T, mem_size_datums / mem_size_type) + }; + + Ok(datums) } } From b66204adbba02fcdf5c2afdfe9a606a3fc556a79 Mon Sep 17 00:00:00 2001 From: Marc-Antoine Enriquez Date: Sun, 29 Dec 2019 18:07:36 +0100 Subject: [PATCH 31/57] Improve docs wording & hide try_cast from docs --- pg-extend/src/pg_datum.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pg-extend/src/pg_datum.rs b/pg-extend/src/pg_datum.rs index 9c650065..6660d9d1 100644 --- a/pg-extend/src/pg_datum.rs +++ b/pg-extend/src/pg_datum.rs @@ -94,15 +94,16 @@ pub trait TryFromPgDatum<'s>: Sized { } } - /// Used for force casting + /// Used to perform the actual casting. You should not use this directly, but + /// you should implement it. `datum` is expected to be non-null + #[doc(hidden)] unsafe fn try_cast<'mc>( memory_context: &'mc PgAllocator, datum: Datum, ) -> Result where Self: 's, - 'mc: 's, - ; + 'mc: 's; } impl<'s> TryFromPgDatum<'s> for i16 { From f816edf46efbab4a90b9a474352c05aa4c59914c Mon Sep 17 00:00:00 2001 From: Marc-Antoine Enriquez Date: Sun, 29 Dec 2019 18:36:23 +0100 Subject: [PATCH 32/57] Limit slices support to primitive numbers only --- pg-extend/src/pg_datum.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/pg-extend/src/pg_datum.rs b/pg-extend/src/pg_datum.rs index 6660d9d1..76800425 100644 --- a/pg-extend/src/pg_datum.rs +++ b/pg-extend/src/pg_datum.rs @@ -71,6 +71,14 @@ impl<'mc> PgDatum<'mc> { } } +pub trait PgPrimitiveDatum {} + +impl PgPrimitiveDatum for i16 {} +impl PgPrimitiveDatum for i32 {} +impl PgPrimitiveDatum for i64 {} +impl PgPrimitiveDatum for f32 {} +impl PgPrimitiveDatum for f64 {} + /// A trait that allows for conversions between Postgres Datum types and Rust types. /// /// Only Sized types, that fit in a single Datum, bool, u8 - u64 e.g. Nothing else is @@ -378,7 +386,7 @@ where impl<'s, T> TryFromPgDatum<'s> for &[T] where - T: 's + TryFromPgDatum<'s>, + T: 's + TryFromPgDatum<'s> + PgPrimitiveDatum, { unsafe fn try_cast<'mc>( _: &'mc PgAllocator, @@ -416,6 +424,8 @@ where let datums = std::slice::from_raw_parts(elements as *const Datum, nelems as usize); + // This is where the conversion from `&[Datum]` is done to `&[T]` by a simple type casting, + // however, we should use `T::try_cast(&'mc PgAllocator, Datum)` to ignore nulls let mem_size_datums = std::mem::size_of_val(datums); let datums = if mem_size_datums == 0 { std::slice::from_raw_parts(datums.as_ptr() as *const T, 0) From 02a97da11792c9f0b7a1f831ac4d9fec50d30035 Mon Sep 17 00:00:00 2001 From: Marc-Antoine Enriquez Date: Sun, 29 Dec 2019 19:30:09 +0100 Subject: [PATCH 33/57] Revert wrapper --- pg-extend/src/pg_datum.rs | 287 ++++++++++++++++++-------------------- 1 file changed, 138 insertions(+), 149 deletions(-) diff --git a/pg-extend/src/pg_datum.rs b/pg-extend/src/pg_datum.rs index 76800425..b4ed5a06 100644 --- a/pg-extend/src/pg_datum.rs +++ b/pg-extend/src/pg_datum.rs @@ -71,14 +71,6 @@ impl<'mc> PgDatum<'mc> { } } -pub trait PgPrimitiveDatum {} - -impl PgPrimitiveDatum for i16 {} -impl PgPrimitiveDatum for i32 {} -impl PgPrimitiveDatum for i64 {} -impl PgPrimitiveDatum for f32 {} -impl PgPrimitiveDatum for f64 {} - /// A trait that allows for conversions between Postgres Datum types and Rust types. /// /// Only Sized types, that fit in a single Datum, bool, u8 - u64 e.g. Nothing else is @@ -89,41 +81,22 @@ pub trait TryFromPgDatum<'s>: Sized { memory_context: &'mc PgAllocator, datum: PgDatum<'mc>, ) -> Result - where - Self: 's, - 'mc: 's, - { - if let Some(datum) = datum.0 { - unsafe { - Self::try_cast(memory_context, datum) - } - } else { - Err("datum was NULL") - } - } - - /// Used to perform the actual casting. You should not use this directly, but - /// you should implement it. `datum` is expected to be non-null - #[doc(hidden)] - unsafe fn try_cast<'mc>( - memory_context: &'mc PgAllocator, - datum: Datum, - ) -> Result where Self: 's, 'mc: 's; } impl<'s> TryFromPgDatum<'s> for i16 { - unsafe fn try_cast<'mc>( - _: &'mc PgAllocator, - datum: Datum, - ) -> Result + fn try_from<'mc>(_: &'mc PgAllocator, datum: PgDatum<'mc>) -> Result where Self: 's, 'mc: 's, { - Ok(datum as i16) + if let Some(datum) = datum.0 { + Ok(datum as i16) + } else { + Err("datum was NULL") + } } } @@ -134,15 +107,16 @@ impl From for PgDatum<'_> { } impl<'s> TryFromPgDatum<'s> for f32 { - unsafe fn try_cast<'mc>( - _: &'mc PgAllocator, - datum: Datum, - ) -> Result + fn try_from<'mc>(_: &'mc PgAllocator, datum: PgDatum<'mc>) -> Result where Self: 's, 'mc: 's, { - Ok(f32::from_bits(datum as u32)) + if let Some(datum) = datum.0 { + Ok(f32::from_bits(datum as u32)) + } else { + Err("datum was NULL") + } } } @@ -153,15 +127,16 @@ impl From for PgDatum<'_> { } impl<'s> TryFromPgDatum<'s> for f64 { - unsafe fn try_cast<'mc>( - _: &'mc PgAllocator, - datum: Datum, - ) -> Result + fn try_from<'mc>(_: &'mc PgAllocator, datum: PgDatum<'mc>) -> Result where Self: 's, 'mc: 's, { - Ok(f64::from_bits(datum as u64)) + if let Some(datum) = datum.0 { + Ok(f64::from_bits(datum as u64)) + } else { + Err("datum was NULL") + } } } @@ -172,15 +147,16 @@ impl From for PgDatum<'_> { } impl<'s> TryFromPgDatum<'s> for i32 { - unsafe fn try_cast<'mc>( - _: &'mc PgAllocator, - datum: Datum, - ) -> Result + fn try_from<'mc>(_: &'mc PgAllocator, datum: PgDatum<'mc>) -> Result where Self: 's, 'mc: 's, { - Ok(datum as i32) + if let Some(datum) = datum.0 { + Ok(datum as i32) + } else { + Err("datum was NULL") + } } } @@ -191,15 +167,16 @@ impl From for PgDatum<'_> { } impl<'s> TryFromPgDatum<'s> for i64 { - unsafe fn try_cast<'mc>( - _: &'mc PgAllocator, - datum: Datum, - ) -> Result + fn try_from<'mc>(_: &'mc PgAllocator, datum: PgDatum<'mc>) -> Result where Self: 's, 'mc: 's, { - Ok(datum as i64) + if let Some(datum) = datum.0 { + Ok(datum as i64) + } else { + Err("datum was NULL") + } } } @@ -228,14 +205,6 @@ impl<'s> TryFromPgDatum<'s> for String { cstr.into_string() .map_err(|_| "String contained non-utf8 data") } - - unsafe fn try_cast<'mc>(_: &'mc PgAllocator, _: Datum) -> Result - where - Self: 's, - 'mc: 's, - { - unimplemented!("Cast is not implemented for this type"); - } } // FIXME: this lifetime is wrong @@ -252,24 +221,27 @@ impl From for PgDatum<'_> { #[deprecated(note = "String is not Zero cost, please use the CString variant")] impl<'s> TryFromPgDatum<'s> for CString { - unsafe fn try_cast<'mc>( - _: &'mc PgAllocator, - datum: Datum, - ) -> Result + fn try_from<'mc>(_: &'mc PgAllocator, datum: PgDatum<'mc>) -> Result where Self: 's, 'mc: 's, { - let text_val = datum as *const pg_sys::text; + if let Some(datum) = datum.0 { + let text_val = datum as *const pg_sys::text; - crate::guard_pg(|| { - let val: *mut c_char = pg_sys::text_to_cstring(text_val); - let cstr = CStr::from_ptr(val).to_owned(); + unsafe { + crate::guard_pg(|| { + let val: *mut c_char = pg_sys::text_to_cstring(text_val); + let cstr = CStr::from_ptr(val).to_owned(); - pg_sys::pfree(val as *mut _); + pg_sys::pfree(val as *mut _); - Ok(cstr) - }) + Ok(cstr) + }) + } + } else { + Err("datum was NULL") + } } } @@ -284,36 +256,43 @@ impl From for PgDatum<'_> { } impl<'s> TryFromPgDatum<'s> for PgAllocated<'s, CString> { - unsafe fn try_cast<'mc>( + fn try_from<'mc>( memory_context: &'mc PgAllocator, - datum: Datum, + datum: PgDatum<'mc>, ) -> Result where Self: 's, 'mc: 's, { - let text_val = datum as *const pg_sys::text; - crate::guard_pg(|| { - // from varlena.c - /* - * text_to_cstring - * - * Create a palloc'd, null-terminated C string from a text value. - * - * We support being passed a compressed or toasted text value. - * This is a bit bogus since such values shouldn't really be referred to as - * "text *", but it seems useful for robustness. If we didn't handle that - * case here, we'd need another routine that did, anyway. - */ - let cstr = pg_sys::text_to_cstring(text_val) as *mut c_char; - - // this is dangerous! it's owned by CString, which is why PgAllocated will - // block the dealloc - //let cstr = CString::from_raw(val); - let allocated = PgAllocated::from_raw(memory_context, cstr); - - Ok(allocated) - }) + if let Some(datum) = datum.0 { + let text_val = datum as *const pg_sys::text; + + unsafe { + crate::guard_pg(|| { + // from varlena.c + /* + * text_to_cstring + * + * Create a palloc'd, null-terminated C string from a text value. + * + * We support being passed a compressed or toasted text value. + * This is a bit bogus since such values shouldn't really be referred to as + * "text *", but it seems useful for robustness. If we didn't handle that + * case here, we'd need another routine that did, anyway. + */ + let cstr = pg_sys::text_to_cstring(text_val) as *mut c_char; + + // this is dangerous! it's owned by CString, which is why PgAllocated will + // block the dealloc + //let cstr = CString::from_raw(val); + let allocated = PgAllocated::from_raw(memory_context, cstr); + + Ok(allocated) + }) + } + } else { + Err("datum was NULL") + } } } @@ -325,17 +304,21 @@ impl<'s> From> for PgDatum<'s> { } impl<'s> TryFromPgDatum<'s> for Text<'s> { - unsafe fn try_cast<'mc>( + fn try_from<'mc>( memory_context: &'mc PgAllocator, - datum: Datum, + datum: PgDatum<'mc>, ) -> Result where Self: 's, 'mc: 's, { - let text_ptr = datum as *const pg_sys::text; + if let Some(datum) = datum.0 { + let text_ptr = datum as *const pg_sys::text; - Ok(Text::from_raw(memory_context, text_ptr as *mut _)) + unsafe { Ok(Text::from_raw(memory_context, text_ptr as *mut _)) } + } else { + Err("datum was NULL") + } } } @@ -360,14 +343,6 @@ where Ok(Some(result?)) } - - unsafe fn try_cast<'mc>(_: &'mc PgAllocator, _: Datum) -> Result - where - Self: 's, - 'mc: 's, - { - unimplemented!("Cast is not implemented for this type"); - } } impl<'mc, 's, T> From> for PgDatum<'mc> @@ -384,58 +359,72 @@ where } } +pub trait PgPrimitiveDatum {} + +impl PgPrimitiveDatum for i16 {} +impl PgPrimitiveDatum for i32 {} +impl PgPrimitiveDatum for i64 {} +impl PgPrimitiveDatum for f32 {} +impl PgPrimitiveDatum for f64 {} + impl<'s, T> TryFromPgDatum<'s> for &[T] where T: 's + TryFromPgDatum<'s> + PgPrimitiveDatum, { - unsafe fn try_cast<'mc>( + fn try_from<'mc>( _: &'mc PgAllocator, - datum: Datum, + datum: PgDatum<'mc>, ) -> Result where Self: 's, 'mc: 's, { - let datum = datum as *mut pg_sys::varlena; - if datum.is_null() { - return Err("datum was NULL"); - } - - let arr_type = pg_sys::pg_detoast_datum(datum) as *mut pg_sys::ArrayType; - - if (*arr_type).ndim > 1 { - return Err("argument must be empty or one-dimensional array"); - } - - let mut elmlen: pg_sys::int16 = 0; - let mut elmbyval: pg_sys::bool_ = 0; - let mut elmalign: ::std::os::raw::c_char = 0; - - pg_sys::get_typlenbyvalalign((*arr_type).elemtype, &mut elmlen, &mut elmbyval, &mut elmalign); - - let mut nulls = 0 as *mut pg_sys::bool_; - let mut elements = 0 as *mut Datum; - let mut nelems: i32 = 0; - - pg_sys::deconstruct_array(arr_type, (*arr_type).elemtype, - elmlen as i32, elmbyval, elmalign, - &mut elements, &mut nulls, &mut nelems, - ); - - let datums = std::slice::from_raw_parts(elements as *const Datum, nelems as usize); - - // This is where the conversion from `&[Datum]` is done to `&[T]` by a simple type casting, - // however, we should use `T::try_cast(&'mc PgAllocator, Datum)` to ignore nulls - let mem_size_datums = std::mem::size_of_val(datums); - let datums = if mem_size_datums == 0 { - std::slice::from_raw_parts(datums.as_ptr() as *const T, 0) + if let Some(datum) = datum.0 { + unsafe { + let datum = datum as *mut pg_sys::varlena; + if datum.is_null() { + return Err("datum was NULL"); + } + + let arr_type = pg_sys::pg_detoast_datum(datum) as *mut pg_sys::ArrayType; + + if (*arr_type).ndim > 1 { + return Err("argument must be empty or one-dimensional array"); + } + + let mut elmlen: pg_sys::int16 = 0; + let mut elmbyval: pg_sys::bool_ = 0; + let mut elmalign: ::std::os::raw::c_char = 0; + + pg_sys::get_typlenbyvalalign((*arr_type).elemtype, &mut elmlen, &mut elmbyval, &mut elmalign); + + let mut nulls = 0 as *mut pg_sys::bool_; + let mut elements = 0 as *mut Datum; + let mut nelems: i32 = 0; + + pg_sys::deconstruct_array(arr_type, (*arr_type).elemtype, + elmlen as i32, elmbyval, elmalign, + &mut elements, &mut nulls, &mut nelems, + ); + + let datums = std::slice::from_raw_parts(elements as *const Datum, nelems as usize); + + // This is where the conversion from `&[Datum]` is done to `&[T]` by a simple type casting, + // however, we should use `T::try_cast(&'mc PgAllocator, Datum)` to ignore nulls + let mem_size_datums = std::mem::size_of_val(datums); + let datums = if mem_size_datums == 0 { + std::slice::from_raw_parts(datums.as_ptr() as *const T, 0) + } else { + let mem_size_type = std::mem::size_of::(); + assert_eq!(mem_size_datums % mem_size_type, 0); + std::slice::from_raw_parts(datums.as_ptr() as *const T, mem_size_datums / mem_size_type) + }; + + Ok(datums) + } } else { - let mem_size_type = std::mem::size_of::(); - assert_eq!(mem_size_datums % mem_size_type, 0); - std::slice::from_raw_parts(datums.as_ptr() as *const T, mem_size_datums / mem_size_type) - }; - - Ok(datums) + Err("datum was NULL") + } } } From 9478c88618972041ca56344808d173e2db92a2e1 Mon Sep 17 00:00:00 2001 From: Marc-Antoine Enriquez Date: Sun, 29 Dec 2019 19:34:36 +0100 Subject: [PATCH 34/57] Add back missing assertion --- pg-extend/src/pg_datum.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pg-extend/src/pg_datum.rs b/pg-extend/src/pg_datum.rs index b4ed5a06..d0bf080a 100644 --- a/pg-extend/src/pg_datum.rs +++ b/pg-extend/src/pg_datum.rs @@ -173,6 +173,10 @@ impl<'s> TryFromPgDatum<'s> for i64 { 'mc: 's, { if let Some(datum) = datum.0 { + assert!( + std::mem::size_of::() >= std::mem::size_of::(), + "Datum not large enough for i64 values" + ); Ok(datum as i64) } else { Err("datum was NULL") From 8f0d58e58bbfba8ba5ad4ac88aa1cf68765d337b Mon Sep 17 00:00:00 2001 From: Marc-Antoine Enriquez Date: Sun, 29 Dec 2019 19:37:51 +0100 Subject: [PATCH 35/57] Fix missplaced assertion --- pg-extend/src/pg_datum.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pg-extend/src/pg_datum.rs b/pg-extend/src/pg_datum.rs index d0bf080a..3056595d 100644 --- a/pg-extend/src/pg_datum.rs +++ b/pg-extend/src/pg_datum.rs @@ -172,11 +172,11 @@ impl<'s> TryFromPgDatum<'s> for i64 { Self: 's, 'mc: 's, { - if let Some(datum) = datum.0 { - assert!( - std::mem::size_of::() >= std::mem::size_of::(), - "Datum not large enough for i64 values" - ); + assert!( + std::mem::size_of::() >= std::mem::size_of::(), + "Datum not large enough for i64 values" + ); + if let Some(datum) = datum.0 { Ok(datum as i64) } else { Err("datum was NULL") From f394ec1c5a58b5e3d4c8abaac8371b5d346146a1 Mon Sep 17 00:00:00 2001 From: Marc-Antoine Enriquez Date: Sun, 29 Dec 2019 19:39:31 +0100 Subject: [PATCH 36/57] Fix indentation --- pg-extend/src/pg_datum.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pg-extend/src/pg_datum.rs b/pg-extend/src/pg_datum.rs index 3056595d..650a7cdf 100644 --- a/pg-extend/src/pg_datum.rs +++ b/pg-extend/src/pg_datum.rs @@ -172,11 +172,11 @@ impl<'s> TryFromPgDatum<'s> for i64 { Self: 's, 'mc: 's, { - assert!( - std::mem::size_of::() >= std::mem::size_of::(), - "Datum not large enough for i64 values" - ); - if let Some(datum) = datum.0 { + assert!( + std::mem::size_of::() >= std::mem::size_of::(), + "Datum not large enough for i64 values" + ); + if let Some(datum) = datum.0 { Ok(datum as i64) } else { Err("datum was NULL") From 14e3c60ee2b87764b5c46b0b1ccb1aa5f31f8611 Mon Sep 17 00:00:00 2001 From: Marc-Antoine Enriquez Date: Sun, 29 Dec 2019 19:54:03 +0100 Subject: [PATCH 37/57] Comments & doc hidding of new trait --- pg-extend/src/pg_datum.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pg-extend/src/pg_datum.rs b/pg-extend/src/pg_datum.rs index 650a7cdf..ee998d86 100644 --- a/pg-extend/src/pg_datum.rs +++ b/pg-extend/src/pg_datum.rs @@ -363,6 +363,8 @@ where } } +/// Inner trait used to limit which types can be used for direct casting +#[doc(hidden)] pub trait PgPrimitiveDatum {} impl PgPrimitiveDatum for i16 {} From f6c0463f2e490dc05669d8b552d94d1c953bf4fa Mon Sep 17 00:00:00 2001 From: Marc-Antoine Enriquez Date: Mon, 30 Dec 2019 18:28:32 +0100 Subject: [PATCH 38/57] Set bool_ type for macos --- pg-extend/src/pg_sys.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pg-extend/src/pg_sys.rs b/pg-extend/src/pg_sys.rs index 7c7369af..394d8061 100644 --- a/pg-extend/src/pg_sys.rs +++ b/pg-extend/src/pg_sys.rs @@ -29,6 +29,9 @@ include!(concat!(env!("OUT_DIR"), "/postgres.rs")); +#[cfg(target_os = "macos")] +pub type bool_ = ::std::os::raw::c_char; + #[cfg(target_os = "linux")] use std::os::raw::c_int; From a4652184adb6df5daf24afb8ba8c0e8d88d1eb27 Mon Sep 17 00:00:00 2001 From: Marc-Antoine Enriquez Date: Mon, 30 Dec 2019 18:32:51 +0100 Subject: [PATCH 39/57] Revert type override --- pg-extend/src/pg_sys.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/pg-extend/src/pg_sys.rs b/pg-extend/src/pg_sys.rs index 394d8061..7c7369af 100644 --- a/pg-extend/src/pg_sys.rs +++ b/pg-extend/src/pg_sys.rs @@ -29,9 +29,6 @@ include!(concat!(env!("OUT_DIR"), "/postgres.rs")); -#[cfg(target_os = "macos")] -pub type bool_ = ::std::os::raw::c_char; - #[cfg(target_os = "linux")] use std::os::raw::c_int; From a95580fe40930c21969f8629534cae5464d61f33 Mon Sep 17 00:00:00 2001 From: Marc-Antoine Enriquez Date: Mon, 30 Dec 2019 18:38:43 +0100 Subject: [PATCH 40/57] Try conditional override for MacOS on PG12 --- pg-extend/src/pg_sys.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pg-extend/src/pg_sys.rs b/pg-extend/src/pg_sys.rs index 7c7369af..e4fc2fab 100644 --- a/pg-extend/src/pg_sys.rs +++ b/pg-extend/src/pg_sys.rs @@ -29,6 +29,9 @@ include!(concat!(env!("OUT_DIR"), "/postgres.rs")); +#[cfg(all(target_os = "macos", feature = "postgres-12"))] +pub type bool_ = bool; + #[cfg(target_os = "linux")] use std::os::raw::c_int; From 90460142ab87a760d1b02f580c388b795ee64caa Mon Sep 17 00:00:00 2001 From: Marc-Antoine Enriquez Date: Mon, 30 Dec 2019 18:47:15 +0100 Subject: [PATCH 41/57] Init with pgbool!() --- pg-extend/src/pg_datum.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pg-extend/src/pg_datum.rs b/pg-extend/src/pg_datum.rs index ee998d86..6f27dc20 100644 --- a/pg-extend/src/pg_datum.rs +++ b/pg-extend/src/pg_datum.rs @@ -399,7 +399,7 @@ where } let mut elmlen: pg_sys::int16 = 0; - let mut elmbyval: pg_sys::bool_ = 0; + let mut elmbyval = pgbool!(false); let mut elmalign: ::std::os::raw::c_char = 0; pg_sys::get_typlenbyvalalign((*arr_type).elemtype, &mut elmlen, &mut elmbyval, &mut elmalign); From 632b90adf28da10e4f31dd8ce30cdbb7595e76e1 Mon Sep 17 00:00:00 2001 From: Marc-Antoine Enriquez Date: Mon, 30 Dec 2019 18:52:06 +0100 Subject: [PATCH 42/57] bool_ type for pg11 in macos --- pg-extend/src/pg_sys.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pg-extend/src/pg_sys.rs b/pg-extend/src/pg_sys.rs index e4fc2fab..5fa5bd85 100644 --- a/pg-extend/src/pg_sys.rs +++ b/pg-extend/src/pg_sys.rs @@ -29,7 +29,7 @@ include!(concat!(env!("OUT_DIR"), "/postgres.rs")); -#[cfg(all(target_os = "macos", feature = "postgres-12"))] +#[cfg(all(target_os = "macos", any(feature = "postgres-11", feature = "postgres-12")))] pub type bool_ = bool; #[cfg(target_os = "linux")] From e6d7bed0606a63ca3644a2d737d853d29b81a6ea Mon Sep 17 00:00:00 2001 From: Marc-Antoine Enriquez Date: Mon, 30 Dec 2019 19:06:26 +0100 Subject: [PATCH 43/57] Set bool_ type for unix familty --- pg-extend/src/pg_sys.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pg-extend/src/pg_sys.rs b/pg-extend/src/pg_sys.rs index 5fa5bd85..2a05a623 100644 --- a/pg-extend/src/pg_sys.rs +++ b/pg-extend/src/pg_sys.rs @@ -29,7 +29,7 @@ include!(concat!(env!("OUT_DIR"), "/postgres.rs")); -#[cfg(all(target_os = "macos", any(feature = "postgres-11", feature = "postgres-12")))] +#[cfg(all(unix, any(feature = "postgres-11", feature = "postgres-12")))] pub type bool_ = bool; #[cfg(target_os = "linux")] From 6a47166d686ae90af2387ea3e004544d6d0d77f4 Mon Sep 17 00:00:00 2001 From: Marc-Antoine Enriquez Date: Mon, 30 Dec 2019 19:18:20 +0100 Subject: [PATCH 44/57] Make clippy happy --- pg-extend/src/pg_datum.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pg-extend/src/pg_datum.rs b/pg-extend/src/pg_datum.rs index 6f27dc20..50e2d3b8 100644 --- a/pg-extend/src/pg_datum.rs +++ b/pg-extend/src/pg_datum.rs @@ -392,6 +392,7 @@ where return Err("datum was NULL"); } + #[allow(clippy::cast_ptr_alignment)] let arr_type = pg_sys::pg_detoast_datum(datum) as *mut pg_sys::ArrayType; if (*arr_type).ndim > 1 { @@ -404,8 +405,8 @@ where pg_sys::get_typlenbyvalalign((*arr_type).elemtype, &mut elmlen, &mut elmbyval, &mut elmalign); - let mut nulls = 0 as *mut pg_sys::bool_; - let mut elements = 0 as *mut Datum; + let mut nulls = std::ptr::null_mut::(); + let mut elements = std::ptr::null_mut::(); let mut nelems: i32 = 0; pg_sys::deconstruct_array(arr_type, (*arr_type).elemtype, From ec56a44638c67d885febb0f6cef58e727f9d0ee6 Mon Sep 17 00:00:00 2001 From: Marc-Antoine Enriquez Date: Mon, 30 Dec 2019 19:35:56 +0100 Subject: [PATCH 45/57] Cargo fmt --- pg-extend/src/pg_datum.rs | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/pg-extend/src/pg_datum.rs b/pg-extend/src/pg_datum.rs index 50e2d3b8..a6ea9650 100644 --- a/pg-extend/src/pg_datum.rs +++ b/pg-extend/src/pg_datum.rs @@ -377,10 +377,7 @@ impl<'s, T> TryFromPgDatum<'s> for &[T] where T: 's + TryFromPgDatum<'s> + PgPrimitiveDatum, { - fn try_from<'mc>( - _: &'mc PgAllocator, - datum: PgDatum<'mc>, - ) -> Result + fn try_from<'mc>(_: &'mc PgAllocator, datum: PgDatum<'mc>) -> Result where Self: 's, 'mc: 's, @@ -403,15 +400,26 @@ where let mut elmbyval = pgbool!(false); let mut elmalign: ::std::os::raw::c_char = 0; - pg_sys::get_typlenbyvalalign((*arr_type).elemtype, &mut elmlen, &mut elmbyval, &mut elmalign); + pg_sys::get_typlenbyvalalign( + (*arr_type).elemtype, + &mut elmlen, + &mut elmbyval, + &mut elmalign, + ); let mut nulls = std::ptr::null_mut::(); let mut elements = std::ptr::null_mut::(); let mut nelems: i32 = 0; - pg_sys::deconstruct_array(arr_type, (*arr_type).elemtype, - elmlen as i32, elmbyval, elmalign, - &mut elements, &mut nulls, &mut nelems, + pg_sys::deconstruct_array( + arr_type, + (*arr_type).elemtype, + elmlen as i32, + elmbyval, + elmalign, + &mut elements, + &mut nulls, + &mut nelems, ); let datums = std::slice::from_raw_parts(elements as *const Datum, nelems as usize); @@ -424,7 +432,10 @@ where } else { let mem_size_type = std::mem::size_of::(); assert_eq!(mem_size_datums % mem_size_type, 0); - std::slice::from_raw_parts(datums.as_ptr() as *const T, mem_size_datums / mem_size_type) + std::slice::from_raw_parts( + datums.as_ptr() as *const T, + mem_size_datums / mem_size_type, + ) }; Ok(datums) From 59f45d6f71de6b51a785d89d3bb049473038102a Mon Sep 17 00:00:00 2001 From: Marc-Antoine Enriquez Date: Mon, 30 Dec 2019 20:50:18 +0100 Subject: [PATCH 46/57] Remove bogus implementation --- pg-extend/src/pg_datum.rs | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/pg-extend/src/pg_datum.rs b/pg-extend/src/pg_datum.rs index a6ea9650..3a76a0c8 100644 --- a/pg-extend/src/pg_datum.rs +++ b/pg-extend/src/pg_datum.rs @@ -446,18 +446,6 @@ where } } -impl<'mc, 's, T> From<&[T]> for PgDatum<'mc> -where - 'mc: 's, - T: 's, - PgDatum<'mc>: From, -{ - fn from(_value: &[T]) -> Self { - // TODO - PgDatum(None, PhantomData) - } -} - impl From<()> for PgDatum<'static> { fn from(_value: ()) -> Self { PgDatum(None, PhantomData) From 360c479b197d660fa1cd534318e6207b0621f50f Mon Sep 17 00:00:00 2001 From: Marc-Antoine Enriquez Date: Mon, 30 Dec 2019 22:16:23 +0100 Subject: [PATCH 47/57] DetoastedArrayWrapper with drop for memory free --- pg-extend/src/pg_datum.rs | 70 +++++++++++++++++++++++++++++---------- 1 file changed, 53 insertions(+), 17 deletions(-) diff --git a/pg-extend/src/pg_datum.rs b/pg-extend/src/pg_datum.rs index 3a76a0c8..2f9d2fd8 100644 --- a/pg-extend/src/pg_datum.rs +++ b/pg-extend/src/pg_datum.rs @@ -363,6 +363,50 @@ where } } +struct DetoastedArrayWrapper { + original_datum: *mut pg_sys::ArrayType, + arr_type: *mut pg_sys::ArrayType, + elements: *mut Datum, + nulls: *mut pg_sys::bool_ +} + +impl DetoastedArrayWrapper { + unsafe fn detoasted(datum: Datum) -> Result { + let datum = datum as *mut pg_sys::varlena; + if datum.is_null() { + return Err("datum was NULL"); + } + + #[allow(clippy::cast_ptr_alignment)] + let arr_type = pg_sys::pg_detoast_datum(datum) as *mut pg_sys::ArrayType; + + Ok(DetoastedArrayWrapper { + original_datum: datum as *mut pg_sys::ArrayType, + arr_type, + elements: std::ptr::null_mut::(), + nulls: std::ptr::null_mut::() + }) + } +} + +impl Drop for DetoastedArrayWrapper { + fn drop(&mut self) { + if self.arr_type != self.original_datum { + unsafe { + if !self.arr_type.is_null() { + pg_sys::pfree(self.arr_type as *mut _); + } + if !self.elements.is_null() { + pg_sys::pfree(self.elements as *mut _); + } + if !self.nulls.is_null() { + pg_sys::pfree(self.nulls as *mut _); + } + } + } + } +} + /// Inner trait used to limit which types can be used for direct casting #[doc(hidden)] pub trait PgPrimitiveDatum {} @@ -373,7 +417,7 @@ impl PgPrimitiveDatum for i64 {} impl PgPrimitiveDatum for f32 {} impl PgPrimitiveDatum for f64 {} -impl<'s, T> TryFromPgDatum<'s> for &[T] +impl<'s, T> TryFromPgDatum<'s> for &'s [T] where T: 's + TryFromPgDatum<'s> + PgPrimitiveDatum, { @@ -384,15 +428,9 @@ where { if let Some(datum) = datum.0 { unsafe { - let datum = datum as *mut pg_sys::varlena; - if datum.is_null() { - return Err("datum was NULL"); - } - - #[allow(clippy::cast_ptr_alignment)] - let arr_type = pg_sys::pg_detoast_datum(datum) as *mut pg_sys::ArrayType; + let mut detoasted_wrapper = DetoastedArrayWrapper::detoasted(datum)?; - if (*arr_type).ndim > 1 { + if (*(detoasted_wrapper.arr_type)).ndim > 1 { return Err("argument must be empty or one-dimensional array"); } @@ -401,28 +439,26 @@ where let mut elmalign: ::std::os::raw::c_char = 0; pg_sys::get_typlenbyvalalign( - (*arr_type).elemtype, + (*(detoasted_wrapper.arr_type)).elemtype, &mut elmlen, &mut elmbyval, &mut elmalign, ); - let mut nulls = std::ptr::null_mut::(); - let mut elements = std::ptr::null_mut::(); let mut nelems: i32 = 0; pg_sys::deconstruct_array( - arr_type, - (*arr_type).elemtype, + detoasted_wrapper.arr_type, + (*(detoasted_wrapper.arr_type)).elemtype, elmlen as i32, elmbyval, elmalign, - &mut elements, - &mut nulls, + &mut detoasted_wrapper.elements, + &mut detoasted_wrapper.nulls, &mut nelems, ); - let datums = std::slice::from_raw_parts(elements as *const Datum, nelems as usize); + let datums = std::slice::from_raw_parts(detoasted_wrapper.elements as *const Datum, nelems as usize); // This is where the conversion from `&[Datum]` is done to `&[T]` by a simple type casting, // however, we should use `T::try_cast(&'mc PgAllocator, Datum)` to ignore nulls From 5a75777f07798cd5704f7bc13cdc3337ebb0ec94 Mon Sep 17 00:00:00 2001 From: Marc-Antoine Enriquez Date: Mon, 30 Dec 2019 22:31:31 +0100 Subject: [PATCH 48/57] Make clippy happy --- pg-extend/src/pg_datum.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pg-extend/src/pg_datum.rs b/pg-extend/src/pg_datum.rs index 2f9d2fd8..e635e7ec 100644 --- a/pg-extend/src/pg_datum.rs +++ b/pg-extend/src/pg_datum.rs @@ -380,8 +380,11 @@ impl DetoastedArrayWrapper { #[allow(clippy::cast_ptr_alignment)] let arr_type = pg_sys::pg_detoast_datum(datum) as *mut pg_sys::ArrayType; + #[allow(clippy::cast_ptr_alignment)] + let original_datum = datum as *mut pg_sys::ArrayType; + Ok(DetoastedArrayWrapper { - original_datum: datum as *mut pg_sys::ArrayType, + original_datum, arr_type, elements: std::ptr::null_mut::(), nulls: std::ptr::null_mut::() From ecda15741885cbbea6000a01dd6ce13fa9d2dd07 Mon Sep 17 00:00:00 2001 From: Marc-Antoine Enriquez Date: Mon, 30 Dec 2019 22:44:45 +0100 Subject: [PATCH 49/57] Fix format --- pg-extend/src/pg_datum.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/pg-extend/src/pg_datum.rs b/pg-extend/src/pg_datum.rs index e635e7ec..7a71a805 100644 --- a/pg-extend/src/pg_datum.rs +++ b/pg-extend/src/pg_datum.rs @@ -367,7 +367,7 @@ struct DetoastedArrayWrapper { original_datum: *mut pg_sys::ArrayType, arr_type: *mut pg_sys::ArrayType, elements: *mut Datum, - nulls: *mut pg_sys::bool_ + nulls: *mut pg_sys::bool_, } impl DetoastedArrayWrapper { @@ -387,7 +387,7 @@ impl DetoastedArrayWrapper { original_datum, arr_type, elements: std::ptr::null_mut::(), - nulls: std::ptr::null_mut::() + nulls: std::ptr::null_mut::(), }) } } @@ -461,7 +461,10 @@ where &mut nelems, ); - let datums = std::slice::from_raw_parts(detoasted_wrapper.elements as *const Datum, nelems as usize); + let datums = std::slice::from_raw_parts( + detoasted_wrapper.elements as *const Datum, + nelems as usize, + ); // This is where the conversion from `&[Datum]` is done to `&[T]` by a simple type casting, // however, we should use `T::try_cast(&'mc PgAllocator, Datum)` to ignore nulls From 0c13ccb0f1a71a9a3fe98c68e33056656382d01f Mon Sep 17 00:00:00 2001 From: Marc-Antoine Enriquez Date: Tue, 31 Dec 2019 00:54:08 +0100 Subject: [PATCH 50/57] Proper generation of create statements with arrays --- pg-extend/src/pg_type.rs | 19 ++++++++++++++++--- pg-extern-attr/src/lib.rs | 4 ++-- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/pg-extend/src/pg_type.rs b/pg-extend/src/pg_type.rs index 83e39fc9..5fb4c399 100644 --- a/pg-extend/src/pg_type.rs +++ b/pg-extend/src/pg_type.rs @@ -91,12 +91,13 @@ impl PgType { } /// Return the string representation of this type - pub fn as_str(self) -> &'static str { + pub fn as_str(self, as_array: bool) -> &'static str { match self { // abstime AbsoluteTime utils/nabstime.h PgType::AbsoluteTime => "abstime", // bigint (int8) int64 postgres.h PgType::BigInt => "bigint", + PgType::Int8 if as_array => "int8[]", PgType::Int8 => "int8", // boolean bool postgres.h (maybe compiler built-in) PgType::Boolean => "boolean", @@ -114,17 +115,21 @@ impl PgType { PgType::Date => "date", // smallint (int2) int16 postgres.h PgType::SmallInt => "smallint", + PgType::Int2 if as_array => "int2[]", PgType::Int2 => "int2", // int2vector int2vector* postgres.h PgType::Int2Vector => "int2vector", // integer (int4) int32 postgres.h PgType::Integer => "integer", + PgType::Int4 if as_array => "int4[]", PgType::Int4 => "int4", // real (float4) float4* postgres.h PgType::Real => "real", + PgType::Float4 if as_array => "float4[]", PgType::Float4 => "float4", // double precision (float8) float8* postgres.h PgType::DoublePrecision => "double precision", + PgType::Float8 if as_array => "float8[]", PgType::Float8 => "float8", // interval Interval* datatype/timestamp.h PgType::Interval => "interval", @@ -166,8 +171,8 @@ impl PgType { } /// Return the String to be used for the RETURNS statement in SQL - pub fn return_stmt(self) -> String { - format!("RETURNS {}", self.as_str()) + pub fn return_stmt(self, as_array: bool) -> String { + format!("RETURNS {}", self.as_str(as_array)) } } @@ -179,6 +184,10 @@ pub trait PgTypeInfo { fn is_option() -> bool { false } + /// for distinguishing array argsuments + fn is_array() -> bool { + false + } } impl PgTypeInfo for f32 { @@ -255,4 +264,8 @@ where fn pg_type() -> PgType { T::pg_type() } + + fn is_array() -> bool { + true + } } diff --git a/pg-extern-attr/src/lib.rs b/pg-extern-attr/src/lib.rs index 0c4d2c07..087db2f8 100644 --- a/pg-extern-attr/src/lib.rs +++ b/pg-extern-attr/src/lib.rs @@ -162,7 +162,7 @@ fn sql_param_types(arg_types: &[Type]) -> (TokenStream, bool) { let sql_name = Ident::new(&format!("sql_{}", i), arg_type.span()); let sql_param = quote!( - #sql_name = pg_extend::pg_type::PgType::from_rust::<#arg_type>().as_str(), + #sql_name = pg_extend::pg_type::PgType::from_rust::<#arg_type>().as_str(<#arg_type>::is_array()), ); tokens.extend(sql_param); @@ -180,7 +180,7 @@ fn sql_return_type(outputs: &syn::ReturnType) -> TokenStream { syn::ReturnType::Type(_, ty) => quote!(#ty), }; - quote_spanned!(ty.span() => pg_extend::pg_type::PgType::from_rust::<#ty>().return_stmt()) + quote_spanned!(ty.span() => pg_extend::pg_type::PgType::from_rust::<#ty>().return_stmt(<#ty>::is_array())) } /// Returns Rust code to figure out if the function takes optional arguments. Functions with From fa59df47f06ef9d0629e96eba572fa2dfb0920f7 Mon Sep 17 00:00:00 2001 From: Marc-Antoine Enriquez Date: Tue, 31 Dec 2019 04:49:17 +0100 Subject: [PATCH 51/57] Integration tests for array sums --- integration-tests/tests/adding_tests.rs | 90 +++++++++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/integration-tests/tests/adding_tests.rs b/integration-tests/tests/adding_tests.rs index b5e9b797..80d33b0f 100644 --- a/integration-tests/tests/adding_tests.rs +++ b/integration-tests/tests/adding_tests.rs @@ -89,3 +89,93 @@ fn test_add_together() { assert_eq!(col, 6); }); } + +#[test] +fn test_sum_array() { + test_in_db("adding", |mut conn| { + let result = conn + .query( + "SELECT sum_array(ARRAY[1, 2, 3])", + &[], + ) + .expect("query failed"); + assert_eq!(result.len(), 1); + + let row = result.get(0).expect("no rows returned"); + let col: i32 = row.get(0); + + assert_eq!(col, 6); + }); +} + +#[test] +fn test_sum_small_array() { + test_in_db("adding", |mut conn| { + let result = conn + .query( + "SELECT sum_small_array(ARRAY[1, 2, 3]::int2[])", + &[], + ) + .expect("query failed"); + assert_eq!(result.len(), 1); + + let row = result.get(0).expect("no rows returned"); + let col: i16 = row.get(0); + + assert_eq!(col, 6); + }); +} + +#[test] +fn test_sum_big_array() { + test_in_db("adding", |mut conn| { + let result = conn + .query( + "SELECT sum_big_array(ARRAY[1, 2, 3]::int8[])", + &[], + ) + .expect("query failed"); + assert_eq!(result.len(), 1); + + let row = result.get(0).expect("no rows returned"); + let col: i64 = row.get(0); + + assert_eq!(col, 6); + }); +} + +#[test] +fn test_sum_float_array() { + test_in_db("adding", |mut conn| { + let result = conn + .query( + "SELECT sum_float_array(ARRAY[1.1, 2.2, 3.3])", + &[], + ) + .expect("query failed"); + assert_eq!(result.len(), 1); + + let row = result.get(0).expect("no rows returned"); + let col: f32 = row.get(0); + + assert_eq!(format!("{:.1}", col), "6.6".to_owned()); + }); +} + +#[test] +fn test_sum_double_array() { + test_in_db("adding", |mut conn| { + let result = conn + .query( + "SELECT sum_double_array(ARRAY[1.1, 2.2, 3.3])", + &[], + ) + .expect("query failed"); + assert_eq!(result.len(), 1); + + let row = result.get(0).expect("no rows returned"); + let col: f64 = row.get(0); + + assert_eq!(format!("{:.1}", col), "6.6".to_owned()); + }); +} From b594ec25d44ae33b1397c4d3cb0656c0df493dc6 Mon Sep 17 00:00:00 2001 From: Marc-Antoine Enriquez Date: Tue, 31 Dec 2019 05:21:15 +0100 Subject: [PATCH 52/57] Cargo fmt --- integration-tests/tests/adding_tests.rs | 25 +++++-------------------- 1 file changed, 5 insertions(+), 20 deletions(-) diff --git a/integration-tests/tests/adding_tests.rs b/integration-tests/tests/adding_tests.rs index 80d33b0f..8d5f40a5 100644 --- a/integration-tests/tests/adding_tests.rs +++ b/integration-tests/tests/adding_tests.rs @@ -94,10 +94,7 @@ fn test_add_together() { fn test_sum_array() { test_in_db("adding", |mut conn| { let result = conn - .query( - "SELECT sum_array(ARRAY[1, 2, 3])", - &[], - ) + .query("SELECT sum_array(ARRAY[1, 2, 3])", &[]) .expect("query failed"); assert_eq!(result.len(), 1); @@ -112,10 +109,7 @@ fn test_sum_array() { fn test_sum_small_array() { test_in_db("adding", |mut conn| { let result = conn - .query( - "SELECT sum_small_array(ARRAY[1, 2, 3]::int2[])", - &[], - ) + .query("SELECT sum_small_array(ARRAY[1, 2, 3]::int2[])", &[]) .expect("query failed"); assert_eq!(result.len(), 1); @@ -130,10 +124,7 @@ fn test_sum_small_array() { fn test_sum_big_array() { test_in_db("adding", |mut conn| { let result = conn - .query( - "SELECT sum_big_array(ARRAY[1, 2, 3]::int8[])", - &[], - ) + .query("SELECT sum_big_array(ARRAY[1, 2, 3]::int8[])", &[]) .expect("query failed"); assert_eq!(result.len(), 1); @@ -148,10 +139,7 @@ fn test_sum_big_array() { fn test_sum_float_array() { test_in_db("adding", |mut conn| { let result = conn - .query( - "SELECT sum_float_array(ARRAY[1.1, 2.2, 3.3])", - &[], - ) + .query("SELECT sum_float_array(ARRAY[1.1, 2.2, 3.3])", &[]) .expect("query failed"); assert_eq!(result.len(), 1); @@ -166,10 +154,7 @@ fn test_sum_float_array() { fn test_sum_double_array() { test_in_db("adding", |mut conn| { let result = conn - .query( - "SELECT sum_double_array(ARRAY[1.1, 2.2, 3.3])", - &[], - ) + .query("SELECT sum_double_array(ARRAY[1.1, 2.2, 3.3])", &[]) .expect("query failed"); assert_eq!(result.len(), 1); From ef1b6913be80143ec3de172ea38cb479c042223e Mon Sep 17 00:00:00 2001 From: Marc-Antoine Enriquez Date: Tue, 31 Dec 2019 15:03:30 +0100 Subject: [PATCH 53/57] Use stdbool.h --- pg-extend/build.rs | 1 - pg-extend/src/pg_datum.rs | 4 ++-- pg-extend/src/pg_sys.rs | 3 --- pg-extend/wrapper.h | 1 + 4 files changed, 3 insertions(+), 6 deletions(-) diff --git a/pg-extend/build.rs b/pg-extend/build.rs index 58618bb0..0ae66e3a 100644 --- a/pg-extend/build.rs +++ b/pg-extend/build.rs @@ -101,7 +101,6 @@ fn get_bindings(pg_include: &str) -> bindgen::Builder { .whitelist_type("varattrib_1b") .whitelist_type("varattrib_4b") .whitelist_type(".*Array.*") - .whitelist_type("bool_") // Whitelist PG-related values .whitelist_var("PG.*") // Whitelist log-level values diff --git a/pg-extend/src/pg_datum.rs b/pg-extend/src/pg_datum.rs index 7a71a805..b1c30fe6 100644 --- a/pg-extend/src/pg_datum.rs +++ b/pg-extend/src/pg_datum.rs @@ -367,7 +367,7 @@ struct DetoastedArrayWrapper { original_datum: *mut pg_sys::ArrayType, arr_type: *mut pg_sys::ArrayType, elements: *mut Datum, - nulls: *mut pg_sys::bool_, + nulls: *mut bool, } impl DetoastedArrayWrapper { @@ -387,7 +387,7 @@ impl DetoastedArrayWrapper { original_datum, arr_type, elements: std::ptr::null_mut::(), - nulls: std::ptr::null_mut::(), + nulls: std::ptr::null_mut::(), }) } } diff --git a/pg-extend/src/pg_sys.rs b/pg-extend/src/pg_sys.rs index 2a05a623..7c7369af 100644 --- a/pg-extend/src/pg_sys.rs +++ b/pg-extend/src/pg_sys.rs @@ -29,9 +29,6 @@ include!(concat!(env!("OUT_DIR"), "/postgres.rs")); -#[cfg(all(unix, any(feature = "postgres-11", feature = "postgres-12")))] -pub type bool_ = bool; - #[cfg(target_os = "linux")] use std::os::raw::c_int; diff --git a/pg-extend/wrapper.h b/pg-extend/wrapper.h index b425a4ad..5335e98b 100644 --- a/pg-extend/wrapper.h +++ b/pg-extend/wrapper.h @@ -5,6 +5,7 @@ // http://opensource.org/licenses/MIT>, at your option. This file may not be // copied, modified, or distributed except according to those terms. +#include "stdbool.h" #include "postgres.h" #include "postgres_ext.h" #include "access/relscan.h" From 349c2c025e67dface19535c1aeffa0075deab6be Mon Sep 17 00:00:00 2001 From: Marc-Antoine Enriquez Date: Tue, 31 Dec 2019 18:53:34 +0100 Subject: [PATCH 54/57] Improve create stmt generation --- pg-extend/src/lib.rs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/pg-extend/src/lib.rs b/pg-extend/src/lib.rs index f722ef35..08f9a22d 100644 --- a/pg-extend/src/lib.rs +++ b/pg-extend/src/lib.rs @@ -273,13 +273,20 @@ macro_rules! pg_create_stmt_bin { #[cfg(target_os = "macos")] const DYLIB_EXT: &str = "dylib"; - #[cfg(target_os = "windows")] - const DYLIB_EXT: &str = "dll"; + #[cfg(unix)] + fn main() { + let lib_name = env!("CARGO_PKG_NAME").replace("-", "_"); + + let lib_path = env::args().nth(1).unwrap_or(format!("target/release/lib{}.{}", lib_name, DYLIB_EXT)); + $( println!("{}", lib::$func(&lib_path)); )* + } + + #[cfg(windows)] fn main() { - const LIB_NAME: &str = env!("CARGO_PKG_NAME"); + let lib_name = env!("CARGO_PKG_NAME").replace("-", "_"); - let lib_path = env::args().nth(1).unwrap_or_else(|| format!("target/release/lib{}.{}", LIB_NAME, DYLIB_EXT)); + let lib_path = env::args().nth(1).unwrap_or(format!("target\\\\release\\\\{}.dll", lib_name)); $( println!("{}", lib::$func(&lib_path)); )* } From a2ea1c91a7f39eec2a40e143724f3b405ef98bfd Mon Sep 17 00:00:00 2001 From: Marc-Antoine Enriquez Date: Fri, 3 Jan 2020 04:00:43 +0100 Subject: [PATCH 55/57] Fix arrays support for Windows builds --- .cargo/config | 5 ++++- README.md | 15 ++++++++++---- examples/adding/src/lib.rs | 2 +- pg-extend/src/log.rs | 40 ++++++++++++++++++++++++++++++++++++++ pg-extern-attr/src/lib.rs | 7 ++----- 5 files changed, 58 insertions(+), 11 deletions(-) diff --git a/.cargo/config b/.cargo/config index 04ff481c..91756080 100644 --- a/.cargo/config +++ b/.cargo/config @@ -4,5 +4,8 @@ [target.'cfg(unix)'] rustflags = "-C link-arg=-undefineddynamic_lookup" +# Uncomment the appropiate line depending on your PG version [target.'cfg(windows)'] -rustflags = "-C link-arg=/FORCE" \ No newline at end of file +#rustflags = "-C link-arg=/FORCE:UNRESOLVED -C link-arg=C:/PROGRA~1/POSTGR~1/10/lib/postgres.lib" +#rustflags = "-C link-arg=/FORCE:UNRESOLVED -C link-arg=C:/PROGRA~1/POSTGR~1/11/lib/postgres.lib" +#rustflags = "-C link-arg=/FORCE:UNRESOLVED -C link-arg=C:/PROGRA~1/POSTGR~1/12/lib/postgres.lib" \ No newline at end of file diff --git a/README.md b/README.md index 761bf167..f9be8f65 100644 --- a/README.md +++ b/README.md @@ -42,14 +42,21 @@ First install Postgres. The build should be able to find the directory for the P `PG_INCLUDE_PATH=[/path/to/postgres]/include/server # e.g. /usr/local/pgsql/include/server` -For the dynamic library to compile, your project should also have `.cargo/config` file with content: +For the dynamic library to compile, your project should also have a `.cargo/config` file. The contents of this file varies based on your platform. +### POSIX (aka `unix` family) ```toml -[target.'cfg(unix)'] +[build] rustflags = "-C link-arg=-undefineddynamic_lookup" +``` + +This informs the linker that some of the symbols for Postgres won't be available until runtime on the dynamic library load. -[target.'cfg(windows)'] -rustflags = "-C link-arg=/FORCE" +### Windows +Building for Windows is somewhat cumbersome as the MSVC linker requires you to set the location of `postgres.lib`, but this will depend both on your installation path as well as your PG version. Here's an example where the installation path is the default one for PG 12: +```toml +[build] +rustflags = "-C link-arg=/FORCE:UNRESOLVED -C link-arg=C:/PROGRA~1/POSTGR~1/12/lib/postgres.lib" ``` This informs the linker that some of the symbols for Postgres won't be available until runtime on the dynamic library load. diff --git a/examples/adding/src/lib.rs b/examples/adding/src/lib.rs index 456e25a7..e44a6da1 100644 --- a/examples/adding/src/lib.rs +++ b/examples/adding/src/lib.rs @@ -62,7 +62,7 @@ fn sum_float_array(arr: &[f32]) -> f32 { arr.iter().sum() } -// Test array of f32 +// Test array of f64 #[pg_extern] fn sum_double_array(arr: &[f64]) -> f64 { arr.iter().sum() diff --git a/pg-extend/src/log.rs b/pg-extend/src/log.rs index 00be350e..fa577514 100644 --- a/pg-extend/src/log.rs +++ b/pg-extend/src/log.rs @@ -50,6 +50,7 @@ //! [`pg_log!`]: ../macro.pg_log.html //! [`Level` enum]: enum.Level.html +#[cfg(not(windows))] use std::ffi::CString; use std::fmt; use std::os::raw::{c_char, c_int}; @@ -97,6 +98,29 @@ pub enum Level { Panic = pg_sys::PANIC as isize, } +impl std::fmt::Display for Level { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + let level = match self { + Level::Debug5 => "DEBUG5", + Level::Debug4 => "DEBUG4", + Level::Debug3 => "DEBUG3", + Level::Debug2 => "DEBUG2", + Level::Debug1 => "DEBUG1", + Level::Log => "LOG", + #[cfg(not(feature = "postgres-9"))] + Level::LogServerOnly => "LOG_SERVER_ONLY", + Level::Info => "INFO", + Level::Notice => "NOTICE", + Level::Warning => "WARNING", + Level::Error => "ERROR", + Level::Fatal => "FATAL", + Level::Panic => "PANIC" + }; + + write!(f, "{}", level) + } +} + impl From for c_int { fn from(level: Level) -> Self { level as isize as c_int @@ -207,6 +231,7 @@ macro_rules! pg_log { // WARNING: this is not part of the crate's public API and is subject to change at any time #[doc(hidden)] +#[cfg(not(windows))] pub fn __private_api_log( args: fmt::Arguments, level: Level, @@ -242,3 +267,18 @@ pub fn __private_api_log( } } } + +// TODO: improve logging and figure out a way to deal with side-effects +#[doc(hidden)] +#[cfg(windows)] +pub fn __private_api_log( + args: fmt::Arguments, + level: Level, + &(_module_path, _file, _line): &(*const c_char, *const c_char, u32), +) { + eprintln!("{}: {}", level, args); + + if level as usize >= Level::Error as usize { + panic!(""); + } +} diff --git a/pg-extern-attr/src/lib.rs b/pg-extern-attr/src/lib.rs index 087db2f8..cc131677 100644 --- a/pg-extern-attr/src/lib.rs +++ b/pg-extern-attr/src/lib.rs @@ -211,11 +211,8 @@ fn sql_function_options(arg_types: &[Type]) -> TokenStream { quote!( { let optional_args = [ #( <#arg_types>::is_option() ),* ]; - if optional_args.iter().all(|&x| x) { "" } - else if !optional_args.iter().any(|&x| x) { " STRICT" } - else { - panic!("Cannot mix Option and non-Option arguments."); - } + if !optional_args.iter().any(|&x| x) { " STRICT" } + else { "" } }, ) } From 0a6ec3779577f35fb32376f239ed9ca472ed8f7e Mon Sep 17 00:00:00 2001 From: Marc-Antoine Enriquez Date: Fri, 3 Jan 2020 05:11:21 +0100 Subject: [PATCH 56/57] Soundness --- pg-extend/src/pg_datum.rs | 73 ++++++++++++++++++++------------------- 1 file changed, 37 insertions(+), 36 deletions(-) diff --git a/pg-extend/src/pg_datum.rs b/pg-extend/src/pg_datum.rs index b1c30fe6..0e1ccf99 100644 --- a/pg-extend/src/pg_datum.rs +++ b/pg-extend/src/pg_datum.rs @@ -371,14 +371,14 @@ struct DetoastedArrayWrapper { } impl DetoastedArrayWrapper { - unsafe fn detoasted(datum: Datum) -> Result { + fn detoasted(datum: Datum) -> Result { let datum = datum as *mut pg_sys::varlena; if datum.is_null() { return Err("datum was NULL"); } #[allow(clippy::cast_ptr_alignment)] - let arr_type = pg_sys::pg_detoast_datum(datum) as *mut pg_sys::ArrayType; + let arr_type = unsafe { pg_sys::pg_detoast_datum(datum) as *mut pg_sys::ArrayType }; #[allow(clippy::cast_ptr_alignment)] let original_datum = datum as *mut pg_sys::ArrayType; @@ -395,16 +395,16 @@ impl DetoastedArrayWrapper { impl Drop for DetoastedArrayWrapper { fn drop(&mut self) { if self.arr_type != self.original_datum { - unsafe { - if !self.arr_type.is_null() { - pg_sys::pfree(self.arr_type as *mut _); - } - if !self.elements.is_null() { - pg_sys::pfree(self.elements as *mut _); - } - if !self.nulls.is_null() { - pg_sys::pfree(self.nulls as *mut _); - } + if !self.arr_type.is_null() { + unsafe { pg_sys::pfree(self.arr_type as *mut _) } + } + + if !self.elements.is_null() { + unsafe { pg_sys::pfree(self.elements as *mut _) } + } + + if !self.nulls.is_null() { + unsafe { pg_sys::pfree(self.nulls as *mut _) } } } } @@ -430,17 +430,18 @@ where 'mc: 's, { if let Some(datum) = datum.0 { - unsafe { - let mut detoasted_wrapper = DetoastedArrayWrapper::detoasted(datum)?; + let mut detoasted_wrapper = DetoastedArrayWrapper::detoasted(datum)?; - if (*(detoasted_wrapper.arr_type)).ndim > 1 { - return Err("argument must be empty or one-dimensional array"); - } + if unsafe { (*(detoasted_wrapper.arr_type)).ndim } > 1 { + return Err("argument must be empty or one-dimensional array"); + } - let mut elmlen: pg_sys::int16 = 0; - let mut elmbyval = pgbool!(false); - let mut elmalign: ::std::os::raw::c_char = 0; + let mut elmlen: pg_sys::int16 = 0; + let mut elmbyval = false; + let mut elmalign: c_char = 0; + let mut nelems = 0; + let datums = unsafe { pg_sys::get_typlenbyvalalign( (*(detoasted_wrapper.arr_type)).elemtype, &mut elmlen, @@ -448,8 +449,6 @@ where &mut elmalign, ); - let mut nelems: i32 = 0; - pg_sys::deconstruct_array( detoasted_wrapper.arr_type, (*(detoasted_wrapper.arr_type)).elemtype, @@ -461,27 +460,29 @@ where &mut nelems, ); - let datums = std::slice::from_raw_parts( + std::slice::from_raw_parts( detoasted_wrapper.elements as *const Datum, nelems as usize, - ); - - // This is where the conversion from `&[Datum]` is done to `&[T]` by a simple type casting, - // however, we should use `T::try_cast(&'mc PgAllocator, Datum)` to ignore nulls - let mem_size_datums = std::mem::size_of_val(datums); - let datums = if mem_size_datums == 0 { - std::slice::from_raw_parts(datums.as_ptr() as *const T, 0) - } else { - let mem_size_type = std::mem::size_of::(); - assert_eq!(mem_size_datums % mem_size_type, 0); + ) + }; + + // This is where the conversion from `&[Datum]` is done to `&[T]` by a simple type casting + let mem_size_datums = std::mem::size_of_val(datums); + let datums = if mem_size_datums == 0 { + unsafe { std::slice::from_raw_parts(datums.as_ptr() as *const T, 0) } + } else { + let mem_size_type = std::mem::size_of::(); + assert_eq!(mem_size_datums % mem_size_type, 0); + + unsafe { std::slice::from_raw_parts( datums.as_ptr() as *const T, mem_size_datums / mem_size_type, ) - }; + } + }; - Ok(datums) - } + Ok(datums) } else { Err("datum was NULL") } From 97ab100b136aadfa85c104e0594c337b32caa1cb Mon Sep 17 00:00:00 2001 From: Marc-Antoine Enriquez Date: Fri, 3 Jan 2020 05:19:48 +0100 Subject: [PATCH 57/57] Rust fmt --- pg-extend/build.rs | 4 ++-- pg-extend/src/lib.rs | 2 +- pg-extend/src/log.rs | 2 +- pg-extend/src/pg_datum.rs | 8 ++------ 4 files changed, 6 insertions(+), 10 deletions(-) diff --git a/pg-extend/build.rs b/pg-extend/build.rs index 0ae66e3a..5820c9a8 100644 --- a/pg-extend/build.rs +++ b/pg-extend/build.rs @@ -16,7 +16,7 @@ use std::process::Command; fn main() { let out_path = PathBuf::from(env::var("OUT_DIR").unwrap()).join("postgres.rs"); - let pg_config = env::var("PG_CONFIG").unwrap_or_else(|_| "pg_config".to_string()); + let pg_config = env::var("PG_CONFIG").unwrap_or("pg_config".to_string()); // Re-run this if wrapper.h changes println!("cargo:rerun-if-changed=wrapper.h"); @@ -163,7 +163,7 @@ fn include_dir(pg_config: &str) -> Result { env::var("PG_INCLUDE_PATH").or_else(|err| { match Command::new(pg_config).arg("--includedir-server").output() { Ok(out) => Ok(String::from_utf8(out.stdout).unwrap().trim().to_string()), - Err(..) => Err(err), + Err(_) => Err(err), } }) } diff --git a/pg-extend/src/lib.rs b/pg-extend/src/lib.rs index 08f9a22d..06198f67 100644 --- a/pg-extend/src/lib.rs +++ b/pg-extend/src/lib.rs @@ -281,7 +281,7 @@ macro_rules! pg_create_stmt_bin { $( println!("{}", lib::$func(&lib_path)); )* } - + #[cfg(windows)] fn main() { let lib_name = env!("CARGO_PKG_NAME").replace("-", "_"); diff --git a/pg-extend/src/log.rs b/pg-extend/src/log.rs index fa577514..7ef50c56 100644 --- a/pg-extend/src/log.rs +++ b/pg-extend/src/log.rs @@ -114,7 +114,7 @@ impl std::fmt::Display for Level { Level::Warning => "WARNING", Level::Error => "ERROR", Level::Fatal => "FATAL", - Level::Panic => "PANIC" + Level::Panic => "PANIC", }; write!(f, "{}", level) diff --git a/pg-extend/src/pg_datum.rs b/pg-extend/src/pg_datum.rs index 0e1ccf99..bedeab7f 100644 --- a/pg-extend/src/pg_datum.rs +++ b/pg-extend/src/pg_datum.rs @@ -473,13 +473,9 @@ where } else { let mem_size_type = std::mem::size_of::(); assert_eq!(mem_size_datums % mem_size_type, 0); + let len = mem_size_datums / mem_size_type; - unsafe { - std::slice::from_raw_parts( - datums.as_ptr() as *const T, - mem_size_datums / mem_size_type, - ) - } + unsafe { std::slice::from_raw_parts(datums.as_ptr() as *const T, len) } }; Ok(datums)