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

Import from CSV into table creates empty row of nulls for header row #136

Closed
ajrcarey opened this issue Mar 25, 2019 · 5 comments

Comments

Projects
None yet
2 participants
@ajrcarey
Copy link

commented Mar 25, 2019

(Sorry to be raising so many issues in such a short timespan. I absolutely love nanoSQL and want to create a wrapper library so it can be used in Google Dart, and I'm discovering issues as part of my unit testing of that wrapper. I hope it's not too annoying!)

Consider the following script using 2.2.2, which imports five identically-shaped CSV files into a table.

<!DOCTYPE html>
<html lang="en">
<head>
    <title>NanoSQL CSV null import test</title>
    <meta charset="utf-8">
    <script src="https://cdn.jsdelivr.net/npm/@nano-sql/core@2.2.2/dist/nano-sql.min.js"></script>
</head>
<body>
</body>
<script>
    nSQL().connect({
        id: 'test',
        mode: 'TEMP',
        tables: [
            {
              name : 'users',
              model : {
                'id:uuid' : {'pk': true},
                'forename:string' : {},
                'surname:string' : {},
                'gender:string' : {},
                'age:int' : {},
                'education:string' : {},
                'occupation:string' : {},
                'salary:int' : {}
              },
              indexes: {
                'forename:string' : {},
                'surname:string' : {},
                'age:int' : {},
                'gender:string' : {},
                'education:string' : {},
                'occupation:string' : {},
                'salary:int' : {}
              }
            }
        ]
    }).then(() => {
        var promises = [];

        for (var i = 1; i <= 5; i ++) {
            promises[i - 1] = fetch(i + '.csv')
                .then(response => response.text())
                .then(csv => nSQL('users').loadCSV(csv));
        }

        Promise.all(promises).then(() => {
            nSQL('users')
                .query('select', ['COUNT(*)'])
                .exec()
                .then(result => console.log(result));

            nSQL('users')
                .query('select')
                .where(user => user.forename == null)
                .exec()
                .then(result => console.log(result));
        });
    });
</script>
</html>

(The sample data is attached to #132 .)

Each of the five files contains a header row and then 100 data rows, for a total of 101 rows. The documentation at https://nanosql.gitbook.io/docs/query/import-and-export#import-csv stipulates that there must be a header row in the file.

Since each file contains 100 data rows, the total number of rows in the database after import should be 500. Since no data row contains any null values, the total number of rows in the database matching a SELECT ... WHERE on a NULL should be zero.

The results of the script show (a) that there are 505 rows, not 500; and (b) there are five rows containing null values, not zero.

My guess is that the header rows in the CSV files are being transformed into rows of nulls in the database.

Additionally - and this is more of a question - I wasn't sure how to query on NULL in a SQL-like manner, which is why I used a closure in this specific case. None of the following possibilities returned a valid result:

.where(['forename', '=', null])
.where(['forename', '=', 'NULL'])
.where(['forename', 'IS NULL'])

Is there a way to query for a null value other than by closure?

@ClickSimply

This comment has been minimized.

Copy link
Owner

commented Mar 25, 2019

Absolutely no problem with these many issues, I can't express how grateful I am that you take the time to not only report the issues but provide full examples to test the issues with. It's extremely helpful!

It looks like the issue is actually the empty line at the bottom of your CSV files. I'm still using the example CSVs provided in the order by issue and there's an empty line at the bottom of each. Still, empty lines shouldn't be imported as rows so the fix is going to be ignoring empty lines in the CSV file.

This will be taken care of with a release later today of 2.2.3.

The proper way to test for null is like this:

.where(["forename", "=", "NULL"])

The relevant code is here. Since javascript has multiple forms of NULL there's a special condition for checking those. When you check for "NULL" nanoSQL will return the row if the column value is undefined, null or an empty string.

@ajrcarey

This comment has been minimized.

Copy link
Author

commented Mar 25, 2019

Oh yes! That makes much more sense (a redundant \n at the end of each CSV file) than the header row being mishandled. Thank you for checking.

I tried your WHERE syntax in the test file and I couldn't get it to work; it returns an empty array for me, rather than the five rows containing nulls. The exact query I am using is:

            nSQL('users')
                .query('select')
                .where(["forename", "=", "NULL"])
                .exec()
                .then(result => console.log(result));

@ClickSimply

This comment has been minimized.

Copy link
Owner

commented Mar 25, 2019

Ah, the issue is you're requesting a NULL value for a secondary index.

Secondary indexes bypass the standard query path and use an optimized one that isn't intended to handle NULL. I'll add some code to handle NULL and have NOT NULL requests bypass the secondary index path completely.

@ClickSimply

This comment has been minimized.

Copy link
Owner

commented Mar 25, 2019

2.2.3 is out, this issue should be resolved.

@ajrcarey

This comment has been minimized.

Copy link
Author

commented Mar 26, 2019

Thank you so much, I confirm 2.2.3 resolves the problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.