kylemaxxwell / rpostgresql

Automatically exported from code.google.com/p/rpostgresql
0 stars 0 forks source link

Using field or table aliases in SQL queries causes R/RPostgreSQL to shut down #1

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Hi!

 Using queries with aliases on fields and/or tables (e.g.: "SELECT a.gid AS
id FROM sptable AS a") forces R to shutdown unexpectedly without issuing
any error message.
 I'm using R 2.8.1 on a WinXP 32-bit machine.

Original issue reported on code.google.com by joao...@gmail.com on 4 Apr 2009 at 10:54

GoogleCodeExporter commented 9 years ago
On Thursday 04 June 2009, Dirk Eddelbuettel wrote:
On 4 June 2009 at 16:17, Dylan Beaudette wrote:
| Hi,
|
| I recently upgraded to R 2.9.0 on linux x86. After doing so, I switched
| to the RPostgreSQL package for interfacing with a postgresql database. I
| am using postgresql  8.3.7.
|
| A query that works from the postgresql terminal is causing a segfault
| when executed from R.
|
| My sessionInfo, the error message, and the R code used to generate the
| error are listed below.
|
| I have noticed that a trivial query (SELECT 1 as value) or other queries
| seem to work fine. It is only when I enable the LEFT JOIN (see below)
| that I get a segfault. Could this be related to the treatment of null
| values?

As per some recent messages on the r-sig-db list, I think that the error is
due to a bug in the handling of 'schema.table' queries.  If you just use
'select ... from table' you're fine.

Not sure if this helps you -- someone has to go in and fix the bug.

Dirk

Thanks Dirk,

After some further investigation, I see that the query works fine if I *do not 
use column aliases* :

# segfaults:
q <- "
SELECT deb_lab_data.* ,
matrix_wet_color_hue as hue, matrix_wet_color_value as value, 
matrix_wet_color_chroma as chroma
FROM deb_lab_data
LEFT JOIN horizon USING (pedon_id, hz_number)
WHERE deb_lab_data.pedon_id ~~ '%SJER%'
ORDER BY deb_lab_data.pedon_id, deb_lab_data.top ASC "

# works fine:
q <- "
SELECT deb_lab_data.* ,
matrix_wet_color_hue, matrix_wet_color_value, 
matrix_wet_color_chroma
FROM deb_lab_data
LEFT JOIN horizon USING (pedon_id, hz_number)
WHERE deb_lab_data.pedon_id ~~ '%SJER%'
ORDER BY deb_lab_data.pedon_id, deb_lab_data.top ASC "

Original comment by ne...@neiltiffin.com on 5 Jun 2009 at 12:45

GoogleCodeExporter commented 9 years ago

Original comment by ne...@neiltiffin.com on 5 Jun 2009 at 12:48

GoogleCodeExporter commented 9 years ago
From Joe Conway,

Looks like *any* query using a column alias will segfault unless the alias 
exactly matches the column name 
(in which case why bother). The 
code starting at line 423 in RS-PostgreSQL.c looks like:

8<-------------------------------
   if(PQftablecol(my_result,j) !=0) {

   /* Code to find whether a row can be nullable or not */
   sprintf(buff,
           "select attnotnull from pg_attribute
            where attrelid=%d and attname='%s'",
           PQftable(my_result,j),(char*)PQfname(my_result,j));
   res = PQexec (conn, buff );

   if(strcmp(PQgetvalue(res,0,0),"f")==0) {
8<-------------------------------
The crash occurs at line 430 (the strcmp()) because PQgetvalue(res,0,0) returns 
NULL.

PQfname() will return the column alias, not the actual column name, therefore 
the PQexec() here returns no 
results. At the very least, 
PQresultStatus(res) or perhaps PQntuples(res) should be used immediately after 
PQexec() to ensure you have a 
good result before trying to use 
it in strcmp().

In any case, I think the simple fix (untested) is something like:

8<-------------------------------
   if(PQftablecol(my_result,j) !=0) {

   /* Code to find whether a row can be nullable or not */
   sprintf(buff,
           "select attnotnull from pg_attribute
            where attrelid=%d and attnum=%d",
           PQftable(my_result,j),PQftablecol(my_result,j));
8<-------------------------------
i.e. use the table column number and pg_attribute.attnum field.

This is beyond what is appropriate for r-help, so I suggest any further 
discussion go off-list (or is there 
somewhere more appropriate, e.g. r-
devel?)

Original comment by ne...@neiltiffin.com on 7 Jun 2009 at 11:43

GoogleCodeExporter commented 9 years ago
I tested the fix in comment #3 and it appears to work.

Editing RS-PostgreSQL.c at line 423:

-----------------------------------------

    if(PQftablecol(my_result,j) !=0) {

        /* Code to find whether a row can be nullable or not */
    sprintf(buff,"select attnotnull from pg_attribute where attrelid=%d and attnum=%d",
            PQftable(my_result,j),PQftablecol(my_result,j));
        res = PQexec (conn, buff );

      if(PQresultStatus(res) == PGRES_TUPLES_OK && PQntuples(res) != 0 &&
         (strcmp(PQgetvalue(res,0,0),"f")==0)) {
        flds->nullOk[j]=(Sint)1;
      } else {
        flds->nullOk[j]=(Sint)0;
      }

        PQclear(res);

    } else {

--------------------------------

That was the only edit i made. I then tarred the files back up, reinstalled, 
and it
works OK:

--------------------------------

>  dbGetQuery (con, "select tag from image_tag")
     tag
1  chips
2 chips2
3  main2
>  dbGetQuery (con, "select tag as foo_longaliasnamehere from image_tag")
  foo_longaliasnamehere
1                 chips
2                chips2
3                 main2
>                                                 
---------------------------------

The above is the extent of my testing though, as I am familiar with LibPQ, but 
know
almost nothing about R or this plug-in.

Original comment by Judd...@gmail.com on 28 Jul 2009 at 2:02

GoogleCodeExporter commented 9 years ago
Hi there,

I just made a sv checkout and the code mentioned above seems to be at line 445
instaead of 423 as said.
Is that okay so far?

Original comment by rali...@gmail.com on 30 Sep 2009 at 11:01

GoogleCodeExporter commented 9 years ago
Yes, I believe so, as there has been some code changes since the comment was 
made.

Original comment by ne...@neiltiffin.com on 30 Sep 2009 at 11:34

GoogleCodeExporter commented 9 years ago

Original comment by ne...@neiltiffin.com on 30 Sep 2009 at 5:14

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
I believe that this was fixed in an earlier patch, and it should be part of 
release 
0.1-5 to be released within the next week.

Original comment by dirk.eddelbuettel on 10 Oct 2009 at 3:53