Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid delete sheet data on writes #67

Open
mharrisb1 opened this issue Feb 11, 2025 · 7 comments · May be fixed by #71
Open

Avoid delete sheet data on writes #67

mharrisb1 opened this issue Feb 11, 2025 · 7 comments · May be fixed by #71

Comments

@mharrisb1
Copy link
Contributor

Not sure if this needs to be an option or if there is a "smarter" way to decide when to clear sheet and when to not in the codebase.

Reference: #66 (comment)

@Alex-Monahan
Copy link
Contributor

Here is my initial thought for what the API might be!

  1. Have a flag (called if_exists) for replace entire sheet, replace range, or append (Based loosely on Pandas to_sql )
  2. Default for COPY TO with no range specified to replace the entire sheet
  3. Default for COPY TO with a range specified to replace just the relevant range

Thoughts on the overall behavior?

@Alex-Monahan
Copy link
Contributor

The Sheets API for clearing data accepts a range argument. So I think all 3 of these cases (delete none, delete range, delete all) are feasible!

https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets.values/clear

@mharrisb1
Copy link
Contributor Author

I like it. Thoughts @archiewood?

Also, do we currently support any COPY TO options? I see this issue #12 to add them. So probably first step regardless of interface for this would be to add options in general.

@archiewood
Copy link
Member

archiewood commented Feb 12, 2025

For the moment, all COPY TO options effectively have to be specified via the URL.
We dont support any options that the user can specify in SQL.

I think this was just an oversight for an early version though, and it would clearly be helpful, e.g.

copy <table_name> to 'https://gsheets/url' (FORMAT gsheet, HEADER True)

I think @Alex-Monahan's suggestions make sense, though I think we should consider the DuckDB copy options where they exist:

https://duckdb.org/docs/sql/statements/copy.html#copy--to-options

  • OVERWRITE (default FALSE)
  • APPEND (default FALSE)

https://duckdb.org/docs/sql/statements/copy.html#csv-options

  • HEADER (default TRUE)

The overwrite and append flags feel like options we could hijack if we wanted

@Alex-Monahan
Copy link
Contributor

Alex-Monahan commented Feb 17, 2025

For the moment, all COPY TO options effectively have to be specified via the URL. We dont support any options that the user can specify in SQL.

I think this was just an oversight for an early version though, and it would clearly be helpful, e.g.

copy <table_name> to 'https://gsheets/url' (FORMAT gsheet, HEADER True)
I think @Alex-Monahan's suggestions make sense, though I think we should consider the DuckDB copy options where they exist:

https://duckdb.org/docs/sql/statements/copy.html#copy--to-options

  • OVERWRITE (default FALSE)
  • APPEND (default FALSE)

https://duckdb.org/docs/sql/statements/copy.html#csv-options

  • HEADER (default TRUE)

The overwrite and append flags feel like options we could hijack if we wanted

I love the idea of reusing parameters! However, I'm not sure exactly how to map the 3 behaviors to those settings. Here is what I'm thinking. Is this intuitive enough?

  1. Clear out entire sheet
    • OVERWRITE TRUE, APPEND FALSE
  2. Clear out just the range that was specified
    • OVERWRITE FALSE, APPEND FALSE
  3. Clear out nothing
    • OVERWRITE FALSE, APPEND TRUE

And we could throw an error if someone tried to run:

  • OVERWRITE TRUE, APPEND TRUE

@Alex-Monahan
Copy link
Contributor

Well, it turns out those built-in names have some requirements:
Binder Error: Can only set one of OVERWRITE_OR_IGNORE, OVERWRITE or APPEND
So, I don't think my initial approach will work.

How about this?

  1. Clear out entire sheet
    * OVERWRITE TRUE (same as APPEND FALSE), ENTIRE_SHEET TRUE
  2. Clear out just the range that was specified
    * OVERWRITE TRUE (same as APPEND FALSE)
  3. Clear out nothing
    * OVERWRITE FALSE (same as APPEND TRUE)

@Alex-Monahan
Copy link
Contributor

Alright, yet another update... It turns out that overwrite and append behave differently than other parameters. So I was not able to figure out how to set them. They just didn't show up in the list of options.

Right now, I have this working with
OVERWRITE_SHEET and OVERWRITE_RANGE. Setting both to false results in append behavior.

That work for now?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants