DB upload fails when names of columns are improper

[saudiwin]

Bug description

Another issue (sorry!), but this one is an easy fix.

If there is a dirty column name added to the SQL table, such as a column name with dots (...), the DBI::sqlInterpolate in the sqlInterpolateList function will fail with an error that it cannot interpolate the values when doing an update. So it would be good to have some sanitization when creating the columns (or checking first before using sqlInterpolate). That is, the Postgres table will allow a dirty name like var1....low, but it can cause other failures downstream.

Steps to reproduce

No response

Expected behavior

No response

Actual behavior

No response

Your environment

No response

[jhelvy]

@pingfan-hu Could you look into this one as well? I think this should be a rather minor fix, maybe just a bump to 0.12.1 if you figure it out.

[pingfan-hu]

Sure, I'll work on it. I cannot overwrite the built-in rules for DB, but I can do some work-around. Maybe I'll force column names to snake case. How's that?

[saudiwin]

Yes I think you want to sanitize the column names before creating columns. The janitor package has a nice convenience function for this & I know there are others as well. All kinds of negative downstream implications if you allow improper column names in SQL.

It seems like R packages like RPostgres allow for improper column names/table names to be created, so you have to enforce it separately.

[jhelvy]

Yeah I think it's a simple call to intercept the names and make them more standardized, though I'm not sure if we would even want to allow strange names at all. Most of the data header names come from the question IDs, so if your ID has some strange thing like q...12, that would just be a terrible choice. So another option is to just run a name screener when you run your survey, and we can throw and error saying the following names are not allowed, etc.

What I want to avoid is surveydown changing the name of some question or page ID to work well for the data header, but then now having to keep track of that change, and also the user getting confused if the name in the data header doesn't match the name they used when defining the question / page. So my preference would be to just run some quick checks on the ID and throw an error requesting the user to fix it.

[saudiwin]

That's fair. I encountered this problem because I ran a loop with sd_store_value and I didn't notice that some of the values fed into the loop were irregular.

The ultimate problem is really SQL; SQL doesn't like weird names, so you would think RPostgres would throw an error, but it doesn't and instead uses some black magic to get Postgres to create the column.

[jhelvy]

Hmm, I see. So yeah you could accidentally create all sorts of strange names with sd_store_value(). I still like the solution being to stop the program and throw and error so that the user has to fix it and use better names, compared to the alternative of the name being sanitized and passed on to SQL. Even if sanitizing it fixes it for SQL, it might cause problems later if the names don't match. So my first instinct is to run a sanitizer, but use it to throw an error and tell the survey designer they need to use a different name.

I'm not sure how best to do that though. The janitor::clean_names() function is amazing, but the package already has a lot of dependencies, and {janitor} has a lot of other things in it, so hopefully there's some base R solution we could develop (or have Claude develop).

[saudiwin]

For sure. The names only need to meet SQL specs. So I'm sure there are existing base R solutions (or GPT as you mentioned).

Maybe add a convenience function to the package for sanitizing and then make it a recommendation when you throw the error?

[jhelvy]

Maybe add a convenience function to the package for sanitizing and then make it a recommendation when you throw the error?

Yeah I like this idea. sd_sanitize_names(), something like that. Then we can use it internally as well and throw and error.