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

repr testing: Change datum creation to row creation to get rid of lifetime problem. #7701

Merged
merged 2 commits into from
Aug 6, 2021

Conversation

wangandi
Copy link
Contributor

@wangandi wangandi commented Aug 5, 2021

As mentioned in #7676,

pub fn get_datum_from_str<'a>(litval: &'a str, littyp: &ScalarType) -> Result<Datum<'a>, String>

has been changed to

pub fn test_spec_to_row<'a, I>(datum_iter: I) -> Result<Row, String>

to get rid of the lifetime issue associated with creating a datum.

I also moved some helpful code from expr-test-util to repr-test-util:

  • If you have a test of the form datum scalar_type, extract_literal_string should automatically parse the datum part out.
  • parse_vec_of_literals allows parsing [vec of data] into Vec<String>. You can then create a Row if you zip the Vec<String> with a Vec`.

@wangandi wangandi requested a review from danhhz August 5, 2021 01:06
Copy link
Contributor

@danhhz danhhz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks! Sorry this took a bit, I've been OOO since you sent it

/// Not all types are supported yet. Currently supported types:
/// * string, bool, timestamp
/// * all flavors of numeric types
pub fn test_spec_to_row<'a, I>(datum_iter: I) -> Result<Row, String>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you want to add another repr_test_tuil/tests/testdata file with some examples of rows? Those have been really useful for me in figuring out what the input to these functions looks like

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure! I added src/repr-test-util/tests/testdata/row.

@wangandi wangandi merged commit ed6b16d into MaterializeInc:main Aug 6, 2021
@wangandi wangandi deleted the rowcreation branch August 6, 2021 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants