nahanni / rw_redis_fdw

Other
96 stars 23 forks source link

Compatibility with PostgreSQL10.x #13

Closed GHNewbiee closed 6 years ago

GHNewbiee commented 6 years ago

Hi,

I tried to compile the extension for pg10.x and I got the following error:

redis_fdw.c:2499:12: error: too many arguments to function ‘ExecEvalExpr’
    datum = ExecEvalExpr(expr_state, econtext,&is_null, NULL);
            ^
In file included from /usr/include/postgresql/10/server/funcapi.h:22:0,
                 from redis_fdw.c:43:
/usr/include/postgresql/10/server/executor/executor.h:285:1: note: declared here
 ExecEvalExpr(ExprState *state,

So, I removed the last argument (NULL) and compiled it successfully.

Because I am not an experienced user, I would like to ask if I take any great risk to damage the DB by keeping on using the extension with pg10.x.

Tia

l-d-x commented 6 years ago

It looks like pgsql 10 does not require the last parameter. I'll need to make another case for version 10 API.

GHNewbiee commented 6 years ago

I think that removing the last parameter is not enough.

I can run any statement only for the first 5 times. After that (6th time) an error always appears: KEY is NULL (redis_fdw.c @ line: 2571).

Does it have to do with argvlen[0] = 5; in line 730?

Tia

l-d-x commented 6 years ago

L:730 is let pgsql know the length of the argument's string.

The key is required to make a query for a hash table, aka hmget. Not sure what's going on. Do you have your table schema and query string that you're trying to use?

GHNewbiee commented 6 years ago

I have used a similar schema to yours:

CREATE FOREIGN TABLE rft_str(
      key    TEXT,
      value  TEXT,
      expiry INT
  ) SERVER xxx
    OPTIONS (tabletype 'string', database '0');

only the table and server names I have changed, and the queries are very simple INSERT or SELECT types. As I have already written, only the first 5 any statements run without problem. From the 6th one the error appears. When I use a function I have to REPLACE it in order I can execute another 5 statements, and go on...

For the time being I use the following manner:

         postgres_fdw           redis_fdw
Pg10.5  <------------>  Pg9.6  <--------->  Redis

which works fine, free of any trouble.

Tia

l-d-x commented 6 years ago

I ran a test on Ubuntu 18 with many insertions and queries it didn't hit your issue. This is with PostgreSQL 10.5 and Redis 4. There might be something else that's causing problems in your setup.

CREATE EXTENSION redis_fdw;

CREATE SERVER redis_server FOREIGN DATA WRAPPER redis_fdw OPTIONS (host '127.0.0.1', port '6379');

CREATE FOREIGN TABLE rft_str (
  key TEXT
  value TEXT,
  expiry INT
) SERVER redis_server OPTIONS(tabletype 'string', database '0');

# many insertions after
INSERT INTO rft_str(key, value) values('k1', 'v1');
...
GHNewbiee commented 6 years ago

Please add expiry values to your multiple inserts, and let me know the result. I have only tested with expiry values being included.

Tia

l-d-x commented 6 years ago

I've just tested it with expiry in the insertion and am not encountering the issue. There's got to be something else going on that could be corrupting your module. I'm going to apply the change to enable support for PostgreSQL 10.

GHNewbiee commented 6 years ago

Great! Tal for your time and sorry for any inconvenience.