Search code examples
javascriptnode.jsreactjspostgresqlnode-postgres

node-postgres dynamic query


I am building a REACT note taking app. I only track the changes the user makes to the note, and not what the current state of the note is. If there are no changes to a specific property, the property will be sent as an empty string. I am handling this in NODE (node-postgres) with the following function:

const updateNote = async (req, res) => {
  const { category, title, body, nid } = req.body;
  let noteStatement = "";
  let valueStatement = "";
  for (const key in req.body)
  {
    if (req.body[key] !== "" && key !== "nid") {
      noteStatement = noteStatement + key + ", ";
      valueStatement = valueStatement + `'${req.body[key]}', `;
    }
  }
  try {
    const result = await pool.query(
      `UPDATE notes SET (${noteStatement}last_updated) 
      = (${valueStatement}(to_timestamp(${Date.now()} / 1000.0))) 
      WHERE nid = ${nid} RETURNING *`
    );
    const note = result.rows;
    return res.status(200).send({ note });
  } catch (err) {
    return res.send({error: err});
  }
};

I may be overthinking, but the goal was to send the smallest bit of data to the DB as possible. I have spent a fair amount of time on this and this is the most pragmatic approach I came up with. Is writing this type of query bad practice? Would it make more sense to send all the note data including properties that have not been updated from React and have a fixed query updating all properties?

EDIT: Updated Query

const updateNote = async (req, res) => {
    const { category, title, body, nid } = req.body;
    const text = `UPDATE notes SET (category, title, body, last_updated) 
            = ($1, $2, $3, (to_timestamp(${Date.now()} / 1000.0))) 
            WHERE nid = $4 RETURNING *`;
    const values = [category, title, body, nid];
    try {
        const result = await pool.query(text, values);
        const note = result.rows;
        return res.status(200).send({ note });
    } catch (err) {
        return res.send({ error: err });
    }
};

Solution

  • I wanted to leave a comment but soon realized I approach the character limit so I would just leave it as a response.

    First of, I want to make sure I understand what you are trying to accomplish. I assume you just wanna update your DB with only the fields that has been provided from the client. With that in mind I just want to underline the fact that most people are trying to overcomplicate things that should not. Everything in software is a tradeoff, in your case your data isn't that big to worry about updating just certain fields. It can be done but not the way you are doing it right now, you can have a utility function that would build a parameterized query based on the values that are not empty/null depending on how would you send the data that did not change from the client

    Which brings me to the 2nd thing, you should never write a SQL query the way you have done it, by concatonating a string, leaves you vulnerable to SQL injections. Instead you must always use parameterized query unless you use some kind of library that abstracts writing the queries (ORMs).

    As a sidenote, never trust data that comes from the client, so always, always validate the data on the server before you make any changes to the DB, even if you already did validation on the client. You can do it using a middleware like celebrate or other validation libraries. Never trust anything that comes from the client.