Fix empty sql statement error

[manasibarate]

Problem
Error in /home/deploy/www/tr_empujar/shared/node_modules/async/lib/async.js:43
Context : If we have a query commented in the sql “transformations” , we get this error.

We need to change snowflake connector to handle this gracefully.

Solution
Added check for empty sql statement error so that process continues on this error after logging the error details to log.

Link to story
https://taskrabbit.atlassian.net/browse/DATA-1411

[rajpalparyani]

Let me know if you want to setup a meeting to discuss the intention of this change .

I am having a hard time understanding how this change will help in this case.

[rajpalparyani]

@manasibarate - I remember adding this statement because there were already some comments in some sql files because of which we get this error, and we should technically not make this as a failure.

Why are we then printing Stack trace for these things ?

[rajpalparyani]

For statements where we get "Empty SQL statement" , the "isEmptySQLStatementError=true;" will always be correct , then it would never go into the "else" part , right ? Am I missing anything here ?

[manasibarate]

@rajpalparyani I had added the logging statements so that we have the information about the error in the logs, removed it now. Also, you are correct, this was missed, the flag would've always remained true. I have reset the flag back to false after the condition is met.

[rajpalparyani]

i still don't understand how this change will help us in this case. In both this PR and current master , we are just calling "callback(null)" in both cases .. The only addition is the flag , which I don't understand how it will help us not get this error.

let's talk more in the scrum as a breakout tomorrow about this.

[manasibarate]

@rajpalparyani : I did some more investigation on this today, adding the if-else flag ensures that the remaining block of code doesnt execute after a callback is executed. The same can be achieved by the above changes, we want to "callback(null)" and "return" once the error condition of "empty SQL statement" is met and not continue execution till the "callback(err)" line.
This solution works too, and is the general way used even in NodeJS source. Example from filesystem module-
Notice the "return callback(...)" -->

fs.fstat(fd, function(er, st) {
if (er) return callback(er);
size = st.size;
if (size === 0) {
// the kernel lies about many files.
// Go ahead and try to read some bytes.
buffers = [];
return read();
}

buffer = new Buffer(size);
read();
});
Following link of stackoverflow summarizes the exact same issue we have-
https://stackoverflow.com/questions/19072228/javascript-callback-which-doesnt-stop-execution-of-lines-below

We can discuss more on this in the breakout.