Stephen-McDaniel / rpostgresql

This repository is an export of the final version from the retired Google Code system (code.google.com/p/rpostgresql).
0 stars 0 forks source link

postgresqlQuoteId change (r152) breaks schema.table references #27

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. in pgsql: create schema foo;
2. dbWriteTable(con,"foo.tmp",data)

What is the expected output? What do you see instead?

I expect a table tmp in schema foo, instead I get the table "foo.tmp" in schema 
public.

What version of the product are you using? On what operating system?
R-2.12/Linux

Please provide any additional information below.

I think this would work (although I did not test it)...

postgresqlQuoteId <- function(identifier){
  ret <- paste('"', gsub('\\.','"."',gsub('"','""',identifier)), '"', sep="")
  ret
}

Original issue reported on code.google.com by jeff.da...@gmail.com on 1 Nov 2010 at 2:22

GoogleCodeExporter commented 9 years ago
dbWriteTable has only a single argument to set for the tablename, so
I think its primary meaning is creating "foo.tmp" in current schema.
If you want to create "tmp" table in "foo" schema
you could try

    dbGetQuery(con, "SET search_path TO foo")
    dbWriteTable(con, "tmp", data)

Another way I can imagine is to create an optional argument of schema for 
dbWriteTable().

Original comment by tomoa...@kenroku.kanazawa-u.ac.jp on 2 Nov 2010 at 5:40

GoogleCodeExporter commented 9 years ago
I tested my change and it works as expected.   Having to change the search_path 
to get the right schema is a pretty painful and error prone way to do it (too 
easy to leave it set wrong).  The sql standard is to reference tables as 
database.schema.table so I think its more reasonable to treat the dots this 
way. 

It is also how RMySQL works (although mysql does not use schemas - it is 
database.table there).

Original comment by jeff.da...@gmail.com on 2 Nov 2010 at 9:20

GoogleCodeExporter commented 9 years ago
I agree that changing the search_path doesn't look nice.

What do you think is the reasonable handling of dbWriteTable(con, "a.b.c.d.e", 
data)?
What is the proper syntax to direct to access "sche.ma"."ta.b.le"?

So, there seems to be no simple way to handle every possibility.
I now have three possible interface in mind

1. optional schema="schema" argument
2. passing a vector of strings c('schema', 'table')
3. optional argument quotetablename=FALSE and ensure the table name supplied
contains no special characters or is quoted properly at the user side.

Original comment by tomoa...@kenroku.kanazawa-u.ac.jp on 2 Nov 2010 at 11:24

GoogleCodeExporter commented 9 years ago
I think it should fail if you give it "a.b.c.d.e" as a table name.  Thats what 
ROracle and RMySQL do (and up until 2 weeks ago so did RPostgreSQL).  Adding 
optional arguments unsupported by the other db drivers would be a mistake when 
supporting the standard sql syntax for schema.table makes interoperating or 
porting much easier.

Original comment by jeff.da...@gmail.com on 2 Nov 2010 at 12:01

GoogleCodeExporter commented 9 years ago
Interoperability to other PostgreSQL application is more important than 
portability of the R code between different database backend that may have 
different features.  

That is; I think fail for "a.b.c.d.e" without preparing means to access to 
"a.b.c.d.e" is a unacceptably bad design.

The conclusion for the capitals consistency discussuion was that arguments for 
dbExistsTable, dbReadTable, dbWriteTable are not a SQL, but the name itself 
before quoting.

If you stick on the portability you will only use capital alphabet for 
identifier and quote always in the SQL statement you write.

The 4th idea is adding an option not to automatically quote identifiers in the 
connection for compatibility.  This option need not specified every time, but 
just specified when the connection is established.  But, I don't want 
personally implement this as this complicates the code with little benefit.  I 
am just saying that I don't object to include if someone implements this.

Note, none of DBI, ROracle, RMySQL, and RPostgreSQL have version number > 1 and 
ROracle and RMySQL may also change in the future, if the authors thinks it 
reasonable.

Perl DBI module defines quote and quote_identifier 
http://search.cpan.org/~timb/DBI/DBI.pm
while the R DBI is yet to define these.  
I assume these are just because the time was not enough to define and implement 
the detail of what and how to quote.

So, I will leave this issue open for someone might have a good 
idea/implementation. But, assign low priority.

Original comment by tomoa...@kenroku.kanazawa-u.ac.jp on 3 Nov 2010 at 1:48

GoogleCodeExporter commented 9 years ago
My idea is to add a postgresTableQuoteName function as

postgresqlTableQuoteName <- function(identifier){
    names <- strsplit(identifier, ".", fixed=TRUE)[[1]]
    if (length(names) == 2){
        res <- paste(postgresqlQuoteId(names[1]),".", postgresqlQuoteId(names[2]),sep="");
    }else{
        res <- postgresqlQuoteId(identifier);
    }
    return res;
}

then change the following lines of code in the postgresqlWriteTable as
1.
    change 
        sql1 <- paste("create table ", postgresqlQuoteId(name), "\n(\n\t", sep="")
        to 
        sql1 <- paste("create table ", postgresqlTableQuoteName(name), "\n(\n\t", sep="")
2.

change     
    sql4 <- paste("COPY", postgresqlQuoteId(name), "FROM STDIN")
 to 

    sql4 <- paste("COPY", postgresqlTableQuoteName(name), "FROM STDIN")

Original comment by guxiaobo...@gmail.com on 6 Mar 2011 at 4:47

GoogleCodeExporter commented 9 years ago
add if length(names) > 2 we should fail the function.

Original comment by guxiaobo...@gmail.com on 6 Mar 2011 at 4:52

GoogleCodeExporter commented 9 years ago
Close this issue as with the new spec,
dbWriteTable(con, c("foo","tmp"), data)
should be used to write to a table tmp in schema foo.

Original comment by tomoa...@kenroku.kanazawa-u.ac.jp on 10 Oct 2011 at 7:35