db hint

[vitaly-t]

two problems in such code: https://github.com/roaiven/ello/blob/22fea3e6e75d61fa0e71ff3f289d178edfa50a90/server/db/index.js

• you should use sql files via QueryFile
• you should not create loose promise requests, ones without .than + .catch.

[1ven]

Hello. Thanks for tips!
2. It better to create tables this way?
db.query(boards).then(() => db.query(lists)).then(() => db.query(cards));

[vitaly-t]

No, you should use a transaction instead, if you change data, or a task, if you are reading data only.

[1ven]

Ok, Thanks!

[vitaly-t]

I noticed from here: https://github.com/roaiven/ello/blob/develop/server/helpers.js

you use:

params: {
            schema: 'public'
        }

but you don't really use variable ${schema} in any of your SQL, which makes that parameter pointless ;)

See QueryFile documentation ;)

[1ven]

Thanks for tip:)

[vitaly-t]

This looks wrong in so many ways... :)

        const columns =  _.keys(props).join();
        const values = _.values(props).join();

        return db.one(
            'INSERT INTO $1~($2~) VALUES($3) RETURNING *',
            [this.table, columns, values]
        );

this can't work, $2~ wraps one column name, you can't use values like this without escaping them`, and you can't insert value via one variable, they will end up broken.

For example, for columns you could use:

const columns =  _.keys(props).map(k=>pgp.as.name(k));

and then use variable $2^ to inject it.

[1ven]

Hello. Yes, It worked for me, because I've been inserting entries only with one column :)
As I understand, also I need to insert values as csv type.
Working example:

const columns = _.keys(props).map(k => pgp.as.name(k)).join();
const values = _.values(props);

return db.one(
    'INSERT INTO $1~($2^) VALUES($3:csv) RETURNING *',
    [this.table, columns, values]
);

[vitaly-t]

That looks right now ;) almost...

const columns = _.keys(props).map(k => pgp.as.name(k)).join(', ');

join should be with ',' ;)

[vitaly-t]

This is super inefficient: https://github.com/roaiven/ello/blob/8d25ac96490d2321e948ed2366d7b460cc0597b8/server/db/index.js

From here: https://github.com/vitaly-t/pg-promise#query-files

You should only create a single instance of QueryFile per file, and then use that instance throughout the application.

And from here: http://vitaly-t.github.io/pg-promise/QueryFile.html

For any given SQL file you should only create a single instance of this class throughout the application.

And you are creating a whole new QueryFile object every time a query is run.

[1ven]

By this way, I am creating tables for my app, if they are not exist.

Each table sql code is placed in a separate file, I need them to be created in a strict order.
But in docs is written, that: You should only create a single instance of QueryFile per file
I'm a little confused. As I understand I need separate file for each sql code?
Or it better to read sql files by fs.readfile() in this case?

How I can create them more effectively?

This way will be inefficient too?

db.tx(function() {
    return this.sequence(index => {
        switch(index) {
            case 0:
                return this.query(sql('cards.sql'));
            case 1:
                return this.query(sql('lists.sql'));
            case 2:
                return this.query(sql('boards.sql'));
            case 3:
                return this.query(sql('users.sql'));
        }
    });
});

[vitaly-t]

Your SQL files are static - right? Just create them statically, when your app starts, and then use those static QueryFile objects ;)

See the example that accompanies QueryFile.

[1ven]

Yes, they are static. I don't need them later in my app.
I use them only for creating tables and nothing more..

I think, I don't need to use QueryFile in this case at all.
If they are static and I don't need to use them later, it will be more simplier to get them by fs.readFileSync and then make a query.

Or I'm wrong?

[vitaly-t]

QueryFile is easier to use either way.