diff --git a/appveyor.yml b/appveyor.yml index 021df2654..1da32ad4e 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -140,11 +140,21 @@ test_script: docker exec -e IGNORES="bgw_db_scheduler" -e TEST_TABLESPACE1_PATH="C:\Users\$env:UserName\Documents\tablespace1\" -e TEST_TABLESPACE2_PATH="C:\Users\$env:UserName\Documents\tablespace2\" -e TEST_SPINWAIT_ITERS=10000 -e USER=postgres -it pgregress /bin/bash -c "cd /timescaledb/build && make regresschecklocal" - #TESTS=drop_extension + $TESTS1 = $? + + # TODO can we switch these to file swapping? + + Add-Content "C:\Program Files\postgresql\10\data\postgresql.conf" "timescaledb.license_key = 'E1eyJlbmRfdGltZSI6IjIwMTgtMTAtMDEgKzAwMDAiLCAic3RhcnRfdGltZSI6IjIwMTgtMDktMDEgKzAwMDAiLCAiaWQiOiI0OTBGQjI2MC1BMjkyLTRBRDktOUFBMi0wMzYwODM1NzkxQjgiLCAia2luZCI6InRyaWFsIn0K'" + + Restart-Service postgresql-x64-10 + + docker exec -e IGNORES="bgw_db_scheduler" -e TEST_TABLESPACE1_PATH="C:\Users\$env:UserName\Documents\tablespace1\" -e TEST_TABLESPACE2_PATH="C:\Users\$env:UserName\Documents\tablespace2\" -e TEST_SPINWAIT_ITERS=10000 -e USER=postgres -it pgregress /bin/bash -c "cd /timescaledb/build && make regresschecklocal-t" + + if( -not $? -or -not $TESTS1 ) { exit 1 } on_failure: - ps: >- - docker exec -it pgregress cat /timescaledb/build/test/regression.diffs /timescaledb/build/test/pgtest/regressions.diffs + docker exec -it pgregress cat /timescaledb/build/test/regression.diffs /timescaledb/build/tsl/test/regression.diffs /timescaledb/build/test/pgtest/regressions.diffs Get-Content -Path "C:\Program Files\postgresql\10\data\pg_log\*" diff --git a/src/chunk.h b/src/chunk.h index 77b4bb5b5..483a38165 100644 --- a/src/chunk.h +++ b/src/chunk.h @@ -75,7 +75,6 @@ extern Chunk *ts_chunk_find(Hyperspace *hs, Point *p); extern List *ts_chunk_find_all_oids(Hyperspace *hs, List *dimension_vecs, LOCKMODE lockmode); extern Chunk *ts_chunk_copy(Chunk *chunk); extern Chunk *ts_chunk_get_by_name_with_memory_context(const char *schema_name, const char *table_name, int16 num_constraints, MemoryContext mctx, bool fail_if_not_found); -extern Chunk *ts_chunk_get_by_relid(Oid relid, int16 num_constraints, bool fail_if_not_found); extern TSDLLEXPORT Chunk *ts_chunk_get_by_id(int32 id, int16 num_constraints, bool fail_if_not_found); extern TSDLLEXPORT Chunk *ts_chunk_get_by_relid(Oid relid, int16 num_constraints, bool fail_if_not_found); extern bool ts_chunk_exists(const char *schema_name, const char *table_name); diff --git a/src/cross_module_fn.c b/src/cross_module_fn.c index 8dbd2b818..1615f2aaa 100644 --- a/src/cross_module_fn.c +++ b/src/cross_module_fn.c @@ -63,7 +63,7 @@ ts_reorder_chunk(PG_FUNCTION_ARGS) */ static void -error_no_default_fn() +error_no_default_fn(void) { ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), @@ -73,7 +73,7 @@ error_no_default_fn() } static bool -error_no_default_fn_bool_void() +error_no_default_fn_bool_void(void) { error_no_default_fn(); return false; diff --git a/src/dimension.c b/src/dimension.c index cc0b2308f..09aa4de20 100644 --- a/src/dimension.c +++ b/src/dimension.c @@ -115,6 +115,8 @@ dimension_type(TupleInfo *ti) return DIMENSION_TYPE_OPEN; elog(ERROR, "invalid partitioning dimension"); + /* suppress compiler warning on MSVC */ + return DIMENSION_TYPE_ANY; } static void diff --git a/src/export.h b/src/export.h index c719e8a63..851d929fb 100644 --- a/src/export.h +++ b/src/export.h @@ -58,7 +58,7 @@ #endif #define TS_FUNCTION_INFO_V1(fn) \ - TSDLLEXPORT Datum fn(PG_FUNCTION_ARGS); \ + PGDLLEXPORT Datum fn(PG_FUNCTION_ARGS); \ PG_FUNCTION_INFO_V1(fn) #endif /* TIMESCALEDB_EXPORT_H */ diff --git a/src/hypertable_restrict_info.c b/src/hypertable_restrict_info.c index db285ffd7..1264739ad 100644 --- a/src/hypertable_restrict_info.c +++ b/src/hypertable_restrict_info.c @@ -209,6 +209,8 @@ dimension_restrict_info_add(DimensionRestrictInfo *dri, int strategy, DimensionV return dimension_restrict_info_closed_add((DimensionRestrictInfoClosed *) dri, strategy, values); default: elog(ERROR, "unknown dimension type: %d", dri->dimension->type); + /* supress compiler warning on MSVC */ + return false; } } diff --git a/src/license_guc.c b/src/license_guc.c index 538000205..86c599500 100644 --- a/src/license_guc.c +++ b/src/license_guc.c @@ -142,15 +142,45 @@ ts_license_update_check(char **newval, void **extra, GucSource source) Assert(tsl_handle != NULL); Assert(tsl_validate_license_fn != NULL); Assert(tsl_startup_fn != NULL); +#ifdef WIN32 + + /* + * freeing the guc extra causes heap corruption on windows so instead we + * re-parse in the assign hook + */ + extra = NULL; +#endif module_can_start = DirectFunctionCall2( tsl_validate_license_fn, CStringGetDatum(*newval), - PointerGetDatum(extra)); + PointerGetDatum(extra) + ); return DatumGetBool(module_can_start); } +#ifdef WIN32 +static void * +revalidate_license(const char *newval) +{ + void *extra = NULL; + void **extra_p = &extra; + + Assert(extra == NULL); + + /* + * Due to windows issues we cannot use the extra parameter. Instead + * re-call the validation function, if we reach this point the license + * must be valid so the function cannot fail cannot fail. + */ + DirectFunctionCall2(tsl_validate_license_fn, + CStringGetDatum(newval), + PointerGetDatum(extra_p)); + return extra; +} +#endif + /* * `ts_license_on_assign` * Links the cross-module function pointers. @@ -161,6 +191,7 @@ ts_license_on_assign(const char *newval, void *extra) if (!can_load) return; + Assert(newval != NULL); Assert(TS_LICENSE_TYPE_IS_VALID(newval)); if (TS_LICENSE_IS_APACHE_ONLY(newval)) { @@ -170,7 +201,14 @@ ts_license_on_assign(const char *newval, void *extra) return; } + Assert(tsl_handle != NULL); + Assert(tsl_startup_fn != NULL); DirectFunctionCall1(tsl_startup_fn, CharGetDatum(0)); +#ifdef WIN32 + Assert(extra == NULL); + extra = revalidate_license(newval); +#endif + Assert(extra != NULL); ts_cm_functions->tsl_license_on_assign(newval, extra); } @@ -245,7 +283,7 @@ loading_failed: /* SQL functions */ -Datum +PGDLLEXPORT Datum ts_tsl_loaded(PG_FUNCTION_ARGS) { if (TS_CURRENT_LICENSE_IS_APACHE_ONLY()) @@ -254,7 +292,7 @@ ts_tsl_loaded(PG_FUNCTION_ARGS) PG_RETURN_BOOL(ts_cm_functions->check_tsl_loaded()); } -Datum +PGDLLEXPORT Datum ts_enterprise_enabled(PG_FUNCTION_ARGS) { if (TS_CURRENT_LICENSE_IS_APACHE_ONLY()) @@ -264,7 +302,7 @@ ts_enterprise_enabled(PG_FUNCTION_ARGS) } -Datum +PGDLLEXPORT Datum ts_current_license_key(PG_FUNCTION_ARGS) { Assert(ts_guc_license_key != NULL); @@ -276,7 +314,7 @@ ts_current_license_key(PG_FUNCTION_ARGS) * Apache only. This function allows us to bypass the test that usually disables * that. */ -Datum +PGDLLEXPORT Datum ts_allow_downgrade_to_apache(PG_FUNCTION_ARGS) { downgrade_to_apache_enabled = true; diff --git a/src/utils.c b/src/utils.c index 3cc19305e..13fea1898 100644 --- a/src/utils.c +++ b/src/utils.c @@ -194,6 +194,9 @@ ts_interval_from_now_to_internal(Datum interval, Oid type_oid) )); } + /* suppress compiler warnings on MSVC */ + Assert(false); + return 0; } /* Returns approximate period in microseconds */ diff --git a/tsl/src/bgw_policy/reorder_api.c b/tsl/src/bgw_policy/reorder_api.c index d0d642a56..67aa8a8e3 100644 --- a/tsl/src/bgw_policy/reorder_api.c +++ b/tsl/src/bgw_policy/reorder_api.c @@ -152,7 +152,12 @@ reorder_remove_policy(PG_FUNCTION_ARGS) errmsg("cannot remove reorder policy, no such policy exists"))); else { - ereport(NOTICE, (errmsg("reorder policy does not exist on hypertable \"%s\", skipping", get_rel_name(hypertable_oid)))); + char *hypertable_name = get_rel_name(hypertable_oid); + + if (hypertable_name != NULL) + ereport(NOTICE, (errmsg("reorder policy does not exist on hypertable \"%s\", skipping", hypertable_name))); + else + ereport(NOTICE, (errmsg("reorder policy does not exist on unnamed hypertable, skipping"))); PG_RETURN_NULL(); } } diff --git a/tsl/src/gapfill/exec.c b/tsl/src/gapfill/exec.c index f3d1d67ac..77d827880 100644 --- a/tsl/src/gapfill/exec.c +++ b/tsl/src/gapfill/exec.c @@ -86,7 +86,7 @@ gapfill_datum_get_internal(Datum value, Oid type) * for other datatypes */ Assert(false); - break; + return 0; } } @@ -116,7 +116,7 @@ gapfill_internal_get_datum(int64 value, Oid type) * for other datatypes */ Assert(false); - break; + return Int64GetDatum(0); } } diff --git a/tsl/src/license.c b/tsl/src/license.c index a3e89a4ec..6a07cdf30 100644 --- a/tsl/src/license.c +++ b/tsl/src/license.c @@ -80,24 +80,32 @@ tsl_license_update_check(PG_FUNCTION_ARGS) bool license_deserialized; char *license_key = NULL; LicenseInfo **guc_extra = NULL; + LicenseInfo license_info = {0}; Assert(!PG_ARGISNULL(0)); Assert(!PG_ARGISNULL(1)); license_key = PG_GETARG_CSTRING(0); guc_extra = (LicenseInfo **) PG_GETARG_POINTER(1); - Assert(guc_extra != NULL); - /* - * According to the postgres guc documentation, string `extra`s MUST be - * allocated with `malloc`. (postgres attempts to `free` unneeded - * guc-extras with the system `free` upon transaction commit, and there's - * no guarantee that any MemoryContext uses the correct allocator.) - */ - *guc_extra = malloc(sizeof(LicenseInfo)); + license_deserialized = license_deserialize_enterprise(license_key, &license_info); + if (guc_extra != NULL) + { + /* + * According to the postgres guc documentation, string `extra`s MUST + * be allocated with `malloc`. (postgres attempts to `free` unneeded + * guc-extras with the system `free` upon transaction commit, and + * there's no guarantee that any MemoryContext uses the correct + * allocator.) However, there is a bug in Windows which causes heap + * corruption if we try to do that. Instead we currently rerun this + * function during the assign, with the assumption that since we + * called the function during the check hook it cannot fail now. + */ + *guc_extra = malloc(sizeof(LicenseInfo)); + memcpy(*guc_extra, &license_info, sizeof(LicenseInfo)); + } - license_deserialized = license_deserialize_enterprise(license_key, *guc_extra); - PG_RETURN_BOOL(license_deserialized && validate_license_info(*guc_extra)); + PG_RETURN_BOOL(license_deserialized && validate_license_info(&license_info)); } /* @@ -106,7 +114,7 @@ tsl_license_update_check(PG_FUNCTION_ARGS) static bool license_deserialize_enterprise(char *license_key, LicenseInfo *license_out) { - LicenseInfo license_temp = {}; + LicenseInfo license_temp = {0}; const LicenseInfo *license_info = NULL; size_t license_key_len = strlen(license_key); diff --git a/tsl/test/expected/tsl_tables.out b/tsl/test/expected/tsl_tables.out index 1cec289b9..7117ffaa6 100644 --- a/tsl/test/expected/tsl_tables.out +++ b/tsl/test/expected/tsl_tables.out @@ -275,7 +275,7 @@ ERROR: relation "fake_table" does not exist at character 30 \set ON_ERROR_STOP 0 -- This should be noop select remove_reorder_policy(2, true); -NOTICE: reorder policy does not exist on hypertable "(null)", skipping +NOTICE: reorder policy does not exist on unnamed hypertable, skipping remove_reorder_policy ----------------------- diff --git a/tsl/test/postgresql.conf b/tsl/test/postgresql.conf index 1dcbb1730..18948938a 100644 --- a/tsl/test/postgresql.conf +++ b/tsl/test/postgresql.conf @@ -1,4 +1,5 @@ shared_preload_libraries=timescaledb max_worker_processes=16 timescaledb.telemetry_level=off +# when changing this, also update the one in appveyor.yml timescaledb.license_key='E1eyJlbmRfdGltZSI6IjIwMTgtMTAtMDEgKzAwMDAiLCAic3RhcnRfdGltZSI6IjIwMTgtMDktMDEgKzAwMDAiLCAiaWQiOiI0OTBGQjI2MC1BMjkyLTRBRDktOUFBMi0wMzYwODM1NzkxQjgiLCAia2luZCI6InRyaWFsIn0K'