adjust / parquet_fdw

Parquet foreign data wrapper for PostgreSQL
PostgreSQL License
333 stars 37 forks source link

Specifying multiple columns in the `sorted` option #20

Closed jnturton closed 4 years ago

jnturton commented 4 years ago

When I specify multiple, space-separated columns, e.g. sorted 'col1 col2 col3', they are not correctly tokenized and the entire string is taken to be a single column name. The following change fixes the problem for me.

diff --git a/parquet_impl.cpp b/parquet_impl.cpp
index 925946a..7438cb9 100644
--- a/parquet_impl.cpp
+++ b/parquet_impl.cpp
@@ -295,7 +295,7 @@ parse_attributes_list(char *start, Oid relid)
 {
     Bitmapset *attrs = NULL;
     char      *token;
-    const char *delim = std::string(" ").c_str(); /* to satisfy g++ compiler */
+    const char *delim = " ";
     AttrNumber attnum;

     while ((token = strtok(start, delim)) != NULL)
zilder commented 4 years ago

Hi @DzamoNorton,

Thanks for your report. You are right, fixed.

PS: There is actually another problem with multiple sorted columns. Currently it only works when columns in sorted option are listed in the same order as they are in the table DDL. It's fixed in the import branch which will be merged soon. Until then if this is an issue for you, i think i can fix this before merge.

jnturton commented 4 years ago

Okay I got lucky then because the column ordering in my table DDL matches my table sort ordering. Do not worry about bringing the fix in ahead of the merge on my account.