Skip to content
Closed
Show file tree
Hide file tree
Changes from all 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
15 changes: 14 additions & 1 deletion .devcontainer/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,18 @@ RUN mkdir .pgenv-staging/
RUN cp -r .pgenv/src .pgenv/pgsql-* .pgenv/config .pgenv-staging/
RUN rm .pgenv-staging/config/default.conf

FROM base AS pg18
RUN MAKEFLAGS="-j $(nproc)" pgenv build 18.0
RUN rm .pgenv/src/*.tar*
RUN make -C .pgenv/src/postgresql-*/ clean
RUN make -C .pgenv/src/postgresql-*/src/include install

# Stage the pgenv artifacts for PG18
RUN mkdir .pgenv-staging/
RUN cp -r .pgenv/src .pgenv/pgsql-* .pgenv/config .pgenv-staging/
RUN rm .pgenv-staging/config/default.conf


FROM base AS uncrustify-builder

RUN sudo apt update && sudo apt install -y cmake tree
Expand Down Expand Up @@ -201,6 +213,7 @@ COPY --link --from=uncrustify-builder /uncrustify/usr/ /usr/
COPY --link --from=pg15 /home/citus/.pgenv-staging/ /home/citus/.pgenv/
COPY --link --from=pg16 /home/citus/.pgenv-staging/ /home/citus/.pgenv/
COPY --link --from=pg17 /home/citus/.pgenv-staging/ /home/citus/.pgenv/
COPY --link --from=pg18 /home/citus/.pgenv-staging/ /home/citus/.pgenv/

COPY --link --from=pipenv /home/citus/.local/share/virtualenvs/ /home/citus/.local/share/virtualenvs/

Expand All @@ -216,7 +229,7 @@ COPY --chown=citus:citus .psqlrc .
RUN sudo chown --from=root:root citus:citus -R ~

# sets default pg version
RUN pgenv switch 17.6
RUN pgenv switch 18.0

# make connecting to the coordinator easy
ENV PGPORT=9700
36 changes: 33 additions & 3 deletions .github/workflows/build_and_test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,12 @@ jobs:
style_checker_image_name: "ghcr.io/citusdata/stylechecker"
style_checker_tools_version: "0.8.18"
sql_snapshot_pg_version: "17.6"
image_suffix: "-va20872f"
image_suffix: "-dev-97072ce"
pg15_version: '{ "major": "15", "full": "15.14" }'
pg16_version: '{ "major": "16", "full": "16.10" }'
pg17_version: '{ "major": "17", "full": "17.6" }'
upgrade_pg_versions: "15.14-16.10-17.6"
pg18_version: '{ "major": "18", "full": "18.0" }'
upgrade_pg_versions: "15.14-16.10-17.6-18.0"
steps:
# Since GHA jobs need at least one step we use a noop step here.
- name: Set up parameters
Expand Down Expand Up @@ -113,6 +114,7 @@ jobs:
- ${{ needs.params.outputs.pg15_version }}
- ${{ needs.params.outputs.pg16_version }}
- ${{ needs.params.outputs.pg17_version }}
- ${{ needs.params.outputs.pg18_version }}
runs-on: ubuntu-latest
container:
image: "${{ matrix.image_name }}:${{ fromJson(matrix.pg_version).full }}${{ matrix.image_suffix }}"
Expand Down Expand Up @@ -144,6 +146,7 @@ jobs:
- ${{ needs.params.outputs.pg15_version }}
- ${{ needs.params.outputs.pg16_version }}
- ${{ needs.params.outputs.pg17_version }}
- ${{ needs.params.outputs.pg18_version }}
make:
- check-split
- check-multi
Expand Down Expand Up @@ -174,6 +177,10 @@ jobs:
pg_version: ${{ needs.params.outputs.pg17_version }}
suite: regress
image_name: ${{ needs.params.outputs.fail_test_image_name }}
- make: check-failure
pg_version: ${{ needs.params.outputs.pg18_version }}
suite: regress
image_name: ${{ needs.params.outputs.fail_test_image_name }}
- make: check-enterprise-failure
pg_version: ${{ needs.params.outputs.pg15_version }}
suite: regress
Expand All @@ -186,6 +193,10 @@ jobs:
pg_version: ${{ needs.params.outputs.pg17_version }}
suite: regress
image_name: ${{ needs.params.outputs.fail_test_image_name }}
- make: check-enterprise-failure
pg_version: ${{ needs.params.outputs.pg18_version }}
suite: regress
image_name: ${{ needs.params.outputs.fail_test_image_name }}
- make: check-pytest
pg_version: ${{ needs.params.outputs.pg15_version }}
suite: regress
Expand All @@ -198,6 +209,10 @@ jobs:
pg_version: ${{ needs.params.outputs.pg17_version }}
suite: regress
image_name: ${{ needs.params.outputs.fail_test_image_name }}
- make: check-pytest
pg_version: ${{ needs.params.outputs.pg18_version }}
suite: regress
image_name: ${{ needs.params.outputs.fail_test_image_name }}
- make: installcheck
suite: cdc
image_name: ${{ needs.params.outputs.test_image_name }}
Expand All @@ -210,6 +225,10 @@ jobs:
suite: cdc
image_name: ${{ needs.params.outputs.test_image_name }}
pg_version: ${{ needs.params.outputs.pg17_version }}
- make: installcheck
suite: cdc
image_name: ${{ needs.params.outputs.test_image_name }}
pg_version: ${{ needs.params.outputs.pg18_version }}
- make: check-query-generator
pg_version: ${{ needs.params.outputs.pg15_version }}
suite: regress
Expand All @@ -222,6 +241,10 @@ jobs:
pg_version: ${{ needs.params.outputs.pg17_version }}
suite: regress
image_name: ${{ needs.params.outputs.fail_test_image_name }}
- make: check-query-generator
pg_version: ${{ needs.params.outputs.pg18_version }}
suite: regress
image_name: ${{ needs.params.outputs.fail_test_image_name }}
runs-on: ubuntu-latest
container:
image: "${{ matrix.image_name }}:${{ fromJson(matrix.pg_version).full }}${{ needs.params.outputs.image_suffix }}"
Expand Down Expand Up @@ -271,6 +294,7 @@ jobs:
- ${{ needs.params.outputs.pg15_version }}
- ${{ needs.params.outputs.pg16_version }}
- ${{ needs.params.outputs.pg17_version }}
- ${{ needs.params.outputs.pg18_version }}
parallel: [0,1,2,3,4,5] # workaround for running 6 parallel jobs
steps:
- uses: actions/checkout@v4
Expand Down Expand Up @@ -321,6 +345,12 @@ jobs:
new_pg_major: 17
- old_pg_major: 15
new_pg_major: 17
- old_pg_major: 17
new_pg_major: 18
- old_pg_major: 16
new_pg_major: 18
- old_pg_major: 15
new_pg_major: 18
env:
old_pg_major: ${{ matrix.old_pg_major }}
new_pg_major: ${{ matrix.new_pg_major }}
Expand Down Expand Up @@ -515,7 +545,7 @@ jobs:
name: Test flakyness
runs-on: ubuntu-latest
container:
image: ${{ needs.params.outputs.fail_test_image_name }}:${{ fromJson(needs.params.outputs.pg17_version).full }}${{ needs.params.outputs.image_suffix }}
image: ${{ needs.params.outputs.fail_test_image_name }}:${{ fromJson(needs.params.outputs.pg18_version).full }}${{ needs.params.outputs.image_suffix }}
options: --user root
env:
runs: 8
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/packaging-test-pipelines.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ jobs:
# Postgres versions are stored in .github/workflows/build_and_test.yml
# file in json strings with major and full keys.
# Below command extracts the versions and get the unique values.
pg_versions=$(cat .github/workflows/build_and_test.yml | grep -oE '"major": "[0-9]+", "full": "[0-9.]+"' | sed -E 's/"major": "([0-9]+)", "full": "([0-9.]+)"/\1/g' | sort | uniq | tr '\n', ',')
pg_versions=$(cat .github/workflows/build_and_test.yml | grep -oE '"major": "[0-9]+", "full": "[^"]+"' | sed -E 's/.*"major": "([0-9]+)".*/\1/' | sort -n | uniq | tr '\n' ',')
pg_versions_array="[ ${pg_versions} ]"
echo "Supported PG Versions: ${pg_versions_array}"
# Below line is needed to set the output variable to be used in the next job
Expand Down
2 changes: 1 addition & 1 deletion configure
Original file line number Diff line number Diff line change
Expand Up @@ -2588,7 +2588,7 @@ fi
if test "$with_pg_version_check" = no; then
{ $as_echo "$as_me:${as_lineno-$LINENO}: building against PostgreSQL $version_num (skipped compatibility check)" >&5
$as_echo "$as_me: building against PostgreSQL $version_num (skipped compatibility check)" >&6;}
elif test "$version_num" != '15' -a "$version_num" != '16' -a "$version_num" != '17'; then
elif test "$version_num" != '15' -a "$version_num" != '16' -a "$version_num" != '17' -a "$version_num" != '18'; then
as_fn_error $? "Citus is not compatible with the detected PostgreSQL version ${version_num}." "$LINENO" 5
else
{ $as_echo "$as_me:${as_lineno-$LINENO}: building against PostgreSQL $version_num" >&5
Expand Down
2 changes: 1 addition & 1 deletion configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ AC_SUBST(with_pg_version_check)

if test "$with_pg_version_check" = no; then
AC_MSG_NOTICE([building against PostgreSQL $version_num (skipped compatibility check)])
elif test "$version_num" != '15' -a "$version_num" != '16' -a "$version_num" != '17'; then
elif test "$version_num" != '15' -a "$version_num" != '16' -a "$version_num" != '17' -a "$version_num" != '18'; then
AC_MSG_ERROR([Citus is not compatible with the detected PostgreSQL version ${version_num}.])
else
AC_MSG_NOTICE([building against PostgreSQL $version_num])
Expand Down
89 changes: 86 additions & 3 deletions src/backend/columnar/columnar_metadata.c
Original file line number Diff line number Diff line change
Expand Up @@ -1993,9 +1993,10 @@ ColumnarNamespaceId(void)
static uint64
LookupStorageId(RelFileLocator relfilelocator)
{
Oid relationId = RelidByRelfilenumber(RelationTablespace_compat(relfilelocator),
RelationPhysicalIdentifierNumber_compat(
relfilelocator));
Oid relationId = RelidByRelfilenumberWithTempTables(RelationTablespace_compat(
relfilelocator),
RelationPhysicalIdentifierNumber_compat(
relfilelocator));

Relation relation = relation_open(relationId, AccessShareLock);
uint64 storageId = ColumnarStorageGetStorageId(relation, false);
Expand Down Expand Up @@ -2123,3 +2124,85 @@ GetFirstRowNumberAttrIndexInColumnarStripe(TupleDesc tupleDesc)
? (Anum_columnar_stripe_first_row_number - 1)
: tupleDesc->natts - 1;
}


/*
* Map a relation's (tablespace, relfilenumber) to a relation's oid and cache
* the result.
* PG18 and latest PG minors have excluded temporary tables from RelidByRelfilenumber,
* so we need to handle them here.
* Relevant PG commit: https://github.com/postgres/postgres/commit/dcdc95cb4
* This function does an extra search if relid is InvalidOid, for temp tables only.
* Code is mostly copy-paste from PG's RelidByRelfilenumber.
* Returns InvalidOid if no relation matching the criteria could be found.
*/
Oid
RelidByRelfilenumberWithTempTables(Oid reltablespace, RelFileNumber relfilenumber)
{
Oid relid;

#if PG_VERSION_NUM >= PG_VERSION_16
relid = RelidByRelfilenumber(reltablespace, relfilenumber);
#else
relid = RelidByRelfilenode(reltablespace, relfilenumber);
#endif

if (relid == InvalidOid)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can this if statement be wrapped in a PG version check for PG18 ? Or is the ignoring of temp tables backported ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nm, saw its backported to PG13.

{
ScanKeyData skey[2];
HeapTuple ntp;

/* pg_class will show 0 when the value is actually MyDatabaseTableSpace */
if (reltablespace == MyDatabaseTableSpace)
{
reltablespace = 0;
}

/* check for plain relations by looking in pg_class */
Relation relation = table_open(RelationRelationId, AccessShareLock);

ScanKeyInit(&skey[0],
Anum_pg_class_reltablespace,
BTEqualStrategyNumber, F_OIDEQ,
ObjectIdGetDatum(reltablespace));
ScanKeyInit(&skey[1],
Anum_pg_class_relfilenode,
BTEqualStrategyNumber, F_OIDEQ,
ObjectIdGetDatum(relfilenumber));

SysScanDesc scandesc = systable_beginscan(relation,
ClassTblspcRelfilenodeIndexId,
true,
NULL,
2,
skey);

bool found = false;

while (HeapTupleIsValid(ntp = systable_getnext(scandesc)))
{
Form_pg_class classform = (Form_pg_class) GETSTRUCT(ntp);

if (found)
{
elog(ERROR,
"unexpected duplicate for tablespace %u, relfilenumber %u",
reltablespace, relfilenumber);
}
found = true;

Assert(classform->reltablespace == reltablespace);
Assert(classform->relfilenode == relfilenumber);

if (classform->relpersistence == RELPERSISTENCE_TEMP)
{
relid = classform->oid;
}
}

systable_endscan(scandesc);
table_close(relation, AccessShareLock);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is there anything to be said for caching the entry and looking up the cache, as RelidByRelfilenode() does ? It extends the duplication, but we'll always scan pg_class for temp tables.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I didn't want to cache the entry since Postgres is no longer caching entries for temp tables. There is the disadvantage of scanning pg_class always for temp tables. The other suggested solution doesn't have the disadvantage #8309
What do you think?

}

return relid;
}
16 changes: 8 additions & 8 deletions src/backend/columnar/columnar_writer.c
Original file line number Diff line number Diff line change
Expand Up @@ -183,10 +183,10 @@ ColumnarWriteRow(ColumnarWriteState *writeState, Datum *columnValues, bool *colu
writeState->stripeSkipList = stripeSkipList;
writeState->compressionBuffer = makeStringInfo();

Oid relationId = RelidByRelfilenumber(RelationTablespace_compat(
writeState->relfilelocator),
RelationPhysicalIdentifierNumber_compat(
writeState->relfilelocator));
Oid relationId = RelidByRelfilenumberWithTempTables(RelationTablespace_compat(
writeState->relfilelocator),
RelationPhysicalIdentifierNumber_compat(
writeState->relfilelocator));
Relation relation = relation_open(relationId, NoLock);
writeState->emptyStripeReservation =
ReserveEmptyStripe(relation, columnCount, chunkRowCount,
Expand Down Expand Up @@ -404,10 +404,10 @@ FlushStripe(ColumnarWriteState *writeState)

elog(DEBUG1, "Flushing Stripe of size %d", stripeBuffers->rowCount);

Oid relationId = RelidByRelfilenumber(RelationTablespace_compat(
writeState->relfilelocator),
RelationPhysicalIdentifierNumber_compat(
writeState->relfilelocator));
Oid relationId = RelidByRelfilenumberWithTempTables(RelationTablespace_compat(
writeState->relfilelocator),
RelationPhysicalIdentifierNumber_compat(
writeState->relfilelocator));
Relation relation = relation_open(relationId, NoLock);

/*
Expand Down
2 changes: 2 additions & 0 deletions src/include/columnar/columnar_metadata.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,5 +65,7 @@ extern List * StripesForRelfilelocator(RelFileLocator relfilelocator);
extern void ColumnarStorageUpdateIfNeeded(Relation rel, bool isUpgrade);
extern List * ExtractColumnarRelOptions(List *inOptions, List **outColumnarOptions);
extern void SetColumnarRelOptions(RangeVar *rv, List *reloptions);
extern Oid RelidByRelfilenumberWithTempTables(Oid reltablespace, RelFileNumber
relfilenumber);

#endif /* COLUMNAR_METADATA_H */
1 change: 0 additions & 1 deletion src/include/pg_version_compat.h
Original file line number Diff line number Diff line change
Expand Up @@ -521,7 +521,6 @@ get_guc_variables_compat(int *gucCount)
#define RelationPhysicalIdentifierBackend_compat(a) (a->smgr_rnode.node)
typedef RelFileNode RelFileLocator;
typedef Oid RelFileNumber;
#define RelidByRelfilenumber(a, b) RelidByRelfilenode(a, b)

#define float_abs(a) Abs(a)

Expand Down
1 change: 1 addition & 0 deletions src/test/regress/citus_tests/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ def get_pg_major_version():
15: "11.1.5",
16: "12.1.5",
17: "13.0.1",
18: "13.2.0",
}

OLDEST_SUPPORTED_CITUS_VERSION = OLDEST_SUPPORTED_CITUS_VERSION_MATRIX[PG_MAJOR_VERSION]
Expand Down
Loading