Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions .devcontainer/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,23 @@ FROM ubuntu:22.04
ENV DEBIAN_FRONTEND="noninteractive"
ENV TZ="Europe/Istanbul"

ARG PG_MAJOR=17
ARG PG_MAJOR=18

# install deps
RUN apt-get update && apt-get -y install build-essential libreadline-dev zlib1g-dev \
flex bison libxml2-dev libxslt-dev libssl-dev \
libxml2-utils xsltproc ccache pkg-config wget \
curl lsb-release ca-certificates gnupg sudo git \
nano net-tools awscli
nano net-tools awscli libkrb5-dev

# install azure-cli
RUN curl -sL https://packages.microsoft.com/keys/microsoft.asc | gpg --dearmor | tee /etc/apt/keyrings/microsoft.gpg > /dev/null
RUN echo "deb [arch=`dpkg --print-architecture` signed-by=/etc/apt/keyrings/microsoft.gpg] https://packages.microsoft.com/repos/azure-cli/ `lsb_release -cs` main" | tee /etc/apt/sources.list.d/azure-cli.list
RUN apt-get update && apt-get install -y azure-cli

# install Postgres
RUN sh -c 'echo "deb https://apt.postgresql.org/pub/repos/apt $(lsb_release -cs)-pgdg main" > /etc/apt/sources.list.d/pgdg.list'
RUN sh -c 'echo "deb https://apt.postgresql.org/pub/repos/apt $(lsb_release -cs)-pgdg main" > /etc/apt/sources.list.d/pgdg.list' && \
sh -c 'echo "deb https://apt.postgresql.org/pub/repos/apt $(lsb_release -cs)-pgdg 18" >> /etc/apt/sources.list.d/pgdg.list'
Copy link
Member Author

Choose a reason for hiding this comment

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

18 is not released under main repo yet

RUN wget --quiet -O - https://www.postgresql.org/media/keys/ACCC4CF8.asc | apt-key add -
RUN apt-get update && apt-get -y install postgresql-${PG_MAJOR}-postgis-3 \
postgresql-server-dev-${PG_MAJOR} \
Expand All @@ -27,11 +28,17 @@ RUN apt-get update && apt-get -y install postgresql-${PG_MAJOR}-postgis-3 \

# set up permissions so that rust user can create extensions
RUN chmod a+rwx `pg_config --pkglibdir` \
`pg_config --pkglibdir`/bitcode \
`pg_config --sharedir`/extension \
/var/run/postgresql/

# install pgaudit
RUN apt-get update && apt-get -y install postgresql-${PG_MAJOR}-pgaudit
# install pgaudit extension
RUN if [ ${PG_MAJOR} != "18" ]; then \
Copy link
Member Author

Choose a reason for hiding this comment

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

pgaudit is not released for 18 yet but has integration branch

apt-get -y install postgresql-${PG_MAJOR}-pgaudit; \
else \
git clone --branch integration https://github.com/pgaudit/pgaudit.git /tmp/pgaudit && \
cd /tmp/pgaudit && make install USE_PGXS=1 PG_CONFIG=pg_config; \
fi

# initdb requires non-root user. This will also be the user that runs the container.
ARG USERNAME=rust
Expand Down
43 changes: 29 additions & 14 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ jobs:
build-and-test:
strategy:
matrix:
postgres: [ 14, 15, 16, 17 ]
postgres: [ 14, 15, 16, 17, 18 ]
runs_on: [ 'ubuntu-22.04', 'ubuntu-22.04-arm' ]
include:
- runs_on: ubuntu-22.04
Expand Down Expand Up @@ -91,24 +91,21 @@ jobs:
curl https://raw.githubusercontent.com/pypa/pipenv/master/get-pipenv.py | python
pipenv install --dev

- name: Install PostgreSQL
- name: Install PostgreSQL and postgis
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
- name: Install PostgreSQL and postgis
- name: Install PostgreSQL and pgaudit

Copy link
Member Author

Choose a reason for hiding this comment

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

the comment seems true. pgaudit is installed below.

run: |
sudo sh -c 'echo "deb https://apt.postgresql.org/pub/repos/apt $(lsb_release -cs)-pgdg main" > /etc/apt/sources.list.d/pgdg.list'
sudo sh -c 'echo "deb https://apt.postgresql.org/pub/repos/apt $(lsb_release -cs)-pgdg 18" >> /etc/apt/sources.list.d/pgdg.list'
wget --quiet -O - https://www.postgresql.org/media/keys/ACCC4CF8.asc | sudo apt-key add -
sudo apt-get update
sudo apt-get -y install build-essential libreadline-dev zlib1g-dev flex bison libxml2-dev \
libxslt-dev libssl-dev libxml2-utils xsltproc ccache pkg-config \
gnupg ca-certificates
sudo apt-get -y install postgresql-${{ env.PG_MAJOR }}-postgis-3 \
gnupg ca-certificates libkrb5-dev
sudo apt-get -y install postgresql-${{ env.PG_MAJOR }}-postgis-3 \
postgresql-server-dev-${{ env.PG_MAJOR }} \
postgresql-client-${{ env.PG_MAJOR }} \
postgresql-client-${{ env.PG_MAJOR }} \
libpq-dev
echo "export PG_MAJOR=${{ env.PG_MAJOR }}" >> $GITHUB_ENV

- name: Install pgaudit extension
run: |
sudo apt-get install -y postgresql-${{ env.PG_MAJOR }}-pgaudit

- name: Install azure-cli
run: |
curl -sL https://packages.microsoft.com/keys/microsoft.asc | gpg --dearmor | sudo tee /etc/apt/keyrings/microsoft.gpg > /dev/null
Expand All @@ -124,27 +121,45 @@ jobs:

- name: Set up permissions for PostgreSQL
run: |
sudo chmod a+rwx $(/usr/lib/postgresql/${{ env.PG_MAJOR }}/bin/pg_config --pkglibdir) \
$(/usr/lib/postgresql/${{ env.PG_MAJOR }}/bin/pg_config --sharedir)/extension \
sudo chmod a+rwx $(${{ env.PG_CONFIG }} --pkglibdir) \
$(${{ env.PG_CONFIG }} --pkglibdir)/bitcode \
$(${{ env.PG_CONFIG }} --sharedir)/extension \
/var/run/postgresql/

- uses: actions/checkout@v4
if: ${{ env.PG_MAJOR == '18' }}
with:
repository: 'pgaudit/pgaudit'
ref: 'integration'
path: pgaudit

- name: Install pgaudit extension for pg18
if: ${{ env.PG_MAJOR == '18' }}
run: |
cd pgaudit && make install USE_PGXS=1 PG_CONFIG=${{ env.PG_CONFIG }}

- name: Install pgaudit extension for < pg18
if: ${{ env.PG_MAJOR != '18' }}
run: |
sudo apt-get -y install postgresql-${{ env.PG_MAJOR }}-pgaudit

- name: Check format and lint
run: |
make check-format
make check-lint

- name: Run tests without coverage
if: ${{ env.PG_MAJOR != '17' || matrix.arch != 'x86_64' }}
if: ${{ env.PG_MAJOR != '18' || matrix.arch != 'x86_64' }}
run: |
make check

- name: Run tests with coverage
if: ${{ env.PG_MAJOR == '17' && matrix.arch == 'x86_64' }}
if: ${{ env.PG_MAJOR == '18' && matrix.arch == 'x86_64' }}
run: |
make check-with-coverage

- name: Upload coverage report to Codecov
if: ${{ env.PG_MAJOR == '17' && matrix.arch == 'x86_64' }}
if: ${{ env.PG_MAJOR == '18' && matrix.arch == 'x86_64' }}
uses: codecov/codecov-action@v4
with:
fail_ci_if_error: true
Expand Down
3 changes: 2 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ name = "pgrx_embed_pg_parquet"
path = "./src/bin/pgrx_embed.rs"

[features]
default = ["pg17"]
default = ["pg18"]
pg18 = ["pgrx/pg18", "pgrx-tests/pg18"]
pg17 = ["pgrx/pg17", "pgrx-tests/pg17"]
pg16 = ["pgrx/pg16", "pgrx-tests/pg16"]
pg15 = ["pgrx/pg15", "pgrx-tests/pg15"]
Expand Down
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ After installing `Postgres`, you need to set up `rustup`, `cargo-pgrx` to build

# set cargo-pgrx (should be the same as pgrx dep in Cargo.toml) and pg versions
> export CARGO_PGRX_VERSION=0.16.0
> export PG_MAJOR=17
> export PG_MAJOR=18

# install cargo-pgrx
> cargo install --force --locked cargo-pgrx@"${CARGO_PGRX_VERSION}"
Expand Down Expand Up @@ -385,3 +385,4 @@ There is currently only one GUC parameter to enable/disable the `pg_parquet`:
| 15 | ✅ |
| 16 | ✅ |
| 17 | ✅ |
| 18 | ✅ |
17 changes: 16 additions & 1 deletion src/parquet_copy_hook/copy_to.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,8 @@ pub(crate) fn execute_copy_to_with_dest_receiver(
portal.as_ptr(),
i64::MAX,
false,
true,
#[cfg(any(feature = "pg14", feature = "pg15", feature = "pg16", feature = "pg17"))]
true, // run_once
parquet_dest.as_ptr(),
parquet_dest.as_ptr(),
&mut completion_tag as _,
Expand Down Expand Up @@ -214,6 +215,20 @@ fn copy_to_stmt_ensure_table_kind(relation: &PgRelation) {
"Try the COPY (SELECT ...) TO variant.",
);
} else if relation_kind == RELKIND_MATVIEW as c_char {
#[cfg(feature = "pg18")]
if !(unsafe { *relation.rd_rel }).relispopulated {
ereport!(
PgLogLevel::ERROR,
PgSqlErrorCode::ERRCODE_FEATURE_NOT_SUPPORTED,
format!(
"cannot copy from unpopulated materialized view \"{}\"",
relation.name()
),
"Use the REFRESH MATERIALIZED VIEW command.",
);
}

#[cfg(any(feature = "pg14", feature = "pg15", feature = "pg16", feature = "pg17"))]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Annoying there is (apparently) no #else-like support here, but I don't know Rust.

Copy link
Member Author

Choose a reason for hiding this comment

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

conditional compilation flags might need care in another PR. It is getting more annoying now that we support 5 Postgres versions.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, probably creating a derived feature/attribute that we can define relative to others in build.rs, say, then testing that in the code location might be an easier approach to maintain.

ereport!(
PgLogLevel::ERROR,
PgSqlErrorCode::ERRCODE_WRONG_OBJECT_TYPE,
Expand Down
8 changes: 4 additions & 4 deletions src/parquet_copy_hook/pg_compat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ pub(crate) fn pg_analyze_and_rewrite(
)
}

#[cfg(any(feature = "pg15", feature = "pg16", feature = "pg17"))]
#[cfg(any(feature = "pg15", feature = "pg16", feature = "pg17", feature = "pg18"))]
Copy link
Collaborator

Choose a reason for hiding this comment

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

would #[cfg(not(feature="pg14"))] be legal/more succinct in this and other later cases?

Copy link
Member Author

Choose a reason for hiding this comment

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

using cfg_aliases now, made feature flags friendlier

unsafe {
pgrx::pg_sys::pg_analyze_and_rewrite_fixedparams(
raw_stmt,
Expand All @@ -50,7 +50,7 @@ pub(crate) fn strVal(val: *mut Node) -> String {
.to_string()
}

#[cfg(any(feature = "pg15", feature = "pg16", feature = "pg17"))]
#[cfg(any(feature = "pg15", feature = "pg16", feature = "pg17", feature = "pg18"))]
unsafe {
let val = (*(val as *mut pgrx::pg_sys::String)).sval;

Expand All @@ -68,15 +68,15 @@ pub(crate) fn MarkGUCPrefixReserved(guc_prefix: &str) {
pgrx::pg_sys::EmitWarningsOnPlaceholders(guc_prefix.as_pg_cstr())
}

#[cfg(any(feature = "pg15", feature = "pg16", feature = "pg17"))]
#[cfg(any(feature = "pg15", feature = "pg16", feature = "pg17", feature = "pg18"))]
unsafe {
pgrx::pg_sys::MarkGUCPrefixReserved(guc_prefix.as_pg_cstr())
}
}

/// check_copy_table_permission checks if the user has permission to copy from/to the table.
/// This is taken from the original PostgreSQL DoCopy function.
#[cfg(any(feature = "pg16", feature = "pg17"))]
#[cfg(any(feature = "pg16", feature = "pg17", feature = "pg18"))]
pub(crate) fn check_copy_table_permission(
p_stmt: &PgBox<PlannedStmt>,
p_state: &PgBox<ParseState>,
Expand Down