Skip to content
This repository has been archived by the owner on Sep 27, 2019. It is now read-only.

Fix CSV scanner for handling filter predicate on a VARCHAR column correctly #1490

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

RonyMin
Copy link

@RonyMin RonyMin commented Nov 30, 2018

CSV scan does not care about delimiter in a CSV file, but query
processor should care the end of each column value, i.e., ‘\0’.

Because query parser parses each selection predicate by adding ‘\0’ at
the end of the predicate value, query processor cannot handle the query
correctly if we ignore the existence of delimiter in a raw tuple in CSV
file.

This simple solution fix the case by replacing delimiter to ‘\0’, and
thus StringCompare in type/type_util.h works correctly.

CSV scan does not care about delimiter in a CSV file, but query
processor should care the end of each column value, i.e., ‘\0’.

Because query parser parses each selection predicate by adding ‘\0’ at
the end of the predicate value, query processor cannot handle the query
correctly if we ignore the existence of delimiter in a raw tuple in CSV
file.

This simple solution fix the case by replacing delimiter to ‘\0’, and
thus StringCompare in type/type_util.h works correctly.
@coveralls
Copy link

coveralls commented Nov 30, 2018

Coverage Status

Coverage increased (+0.02%) to 76.535% when pulling f56beb3 on RonyMin:develop-1 into 3bc6d46 on cmu-db:master.

-----------------------------------------------------------------

/data/system/dev/peloton/src/util/file.cpp: In member function ‘void peloton::util::File::Open(const string&, peloton::util::File::AccessMode)’:
/data/system/dev/peloton/src/util/file.cpp:41:36: error: ‘flags’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
   int fd = open(name.c_str(), flags);
                                    ^
At global scope:
cc1plus: error: unrecognized command line option ‘-Wno-implicit-fallthrough’ [-Werror]
@apavlo
Copy link
Member

apavlo commented Dec 2, 2018

Thanks for sending this. We're not actively developing Peloton anymore. We'll eventually port this over to the new system.

@RonyMin
Copy link
Author

RonyMin commented Dec 3, 2018

@apavlo Hello, professor. It is my pleasure if this PR can improve the new system in any way.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants