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

Understand limitations of current implementation #343

Closed
2 tasks done
krlmlr opened this issue May 1, 2020 · 7 comments
Closed
2 tasks done

Understand limitations of current implementation #343

krlmlr opened this issue May 1, 2020 · 7 comments
Assignees
Milestone

Comments

@krlmlr
Copy link
Collaborator

krlmlr commented May 1, 2020

when compound keys exist in the data model.

On top of #335, or as part of it.

  • Implement compound = FALSE argument to dm_nycflights13().
  • Walk through all functions in https://krlmlr.github.io/dm/dev/reference/. For each function:
    • find an example that uses dm_nycflights13(compound = TRUE)
    • add a verify_output() test to the bottom of the relevant tests/testthat/test-*.R file
      • naked, do not wrap into test_that("...", {})
    • do not attempt to fix
@krlmlr krlmlr added this to the 0.1.3 milestone May 1, 2020
@krlmlr
Copy link
Collaborator Author

krlmlr commented May 5, 2020

Perhaps we need more data models with compound keys in addition. These need to wait until later, we can add tests for everything that can be tested with nycflights13 and FIXMEs for what needs other data models.

@krlmlr krlmlr modified the milestones: 0.1.3, 0.1.4 May 24, 2020
@krlmlr krlmlr modified the milestones: 0.1.4, 0.1.5 Jun 4, 2020
@krlmlr krlmlr modified the milestones: 0.1.6, 0.1.7 Jul 28, 2020
@TSchiefer
Copy link
Member

Currently a limitation is, that dm_nycflights13(compound = TRUE) does not work on PG.
Error is:

Error: Failed to fetch row: ERROR:  column "(origin, time_hour)" does not exist

Should I ignore this for now or investigate/try solving?

@TSchiefer
Copy link
Member

TSchiefer commented Aug 19, 2020

Reason is, that build_copy_data() interprets the key columns (origin, time_hour) as one column and tries to establish unique indices for it.

Should the keys-class maybe be a list that allows entries with length > 1?

And do unique indices work for a compound key at all on a DB?

@krlmlr
Copy link
Collaborator Author

krlmlr commented Aug 19, 2020

Let's not attempt to fix for now.

@krlmlr
Copy link
Collaborator Author

krlmlr commented Aug 19, 2020

When we see the big picture, it becomes clearer where to focus efforts first.

@krlmlr
Copy link
Collaborator Author

krlmlr commented Sep 1, 2020

Done in #436. Need to create issues for next steps.

@krlmlr krlmlr modified the milestones: 0.1.7, 0.1.8 Sep 22, 2020
@krlmlr krlmlr modified the milestones: 0.1.8, 0.1.9, 0.1.10 Nov 18, 2020
@krlmlr krlmlr modified the milestones: 0.1.10, 0.1.11 Jan 6, 2021
@krlmlr krlmlr assigned krlmlr and unassigned TSchiefer Feb 24, 2021
@krlmlr krlmlr modified the milestones: 0.1.11, 0.1.13, 0.2.0 Apr 25, 2021
@krlmlr krlmlr closed this as completed Apr 26, 2021
@github-actions
Copy link
Contributor

This old thread has been automatically locked. If you think you have found something related to this, please open a new issue and link to this old issue if necessary.

@github-actions github-actions bot locked and limited conversation to collaborators Apr 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

2 participants