Skip to content

Deprecate toString() and toQuery() functions #3377

Open

Description

@ben-turner

Environment

Knex version: Latest
Database + version: Any
OS: Any

Feature discussion / request

  1. Explain what is your use case
    The toString() and toQuery() functions don't generate safe SQL queries that can be run against the database. This is mentioned briefly in the docs but is often overlooked. toString() is a pretty standard function in JavaScript—I came across it without seeing it in the docs—and it seems logical that it would output safe SQL if you don't see the warning in the docs.

  2. Explain what kind of feature would support this
    Deprecating these two functions and replacing them with something like toDebugString() will make it immediately clear that it isn't safe to use those functions for anything other than debugging.

  3. Give some API proposal, how the feature should work
    My suggestion is to log a deprecation warning whenever the current functions are called (to avoid breaking existing code by removing them completely) and add a new function called toDebugString() that doesn't log anything and works the same as the existing toString() function.

This issue stemmed from this documentation issue: #5882

Activity

  1. loujaybee commented on Dec 16, 2019

    @loujaybee

    Agree'd I confused another engineer for a while today about why it wasn't emitting the promise object as a string. Is the .toString method overwriting the inherited .toString prototype method? If so I believe that alone would be a strong enough use case to deprecate and replace the name.

  2. ben-turner commented on Dec 17, 2019

    @ben-turner
    Author

    It isn't a promise—it's a query builder. Promises don't have methods like select, where, or join. The best string representation of the query builder is the current query itself, which is what toString outputs. The issue is that many users expect toString to output safe SQL, which it doesn't.

  3. moeiscool commented on Aug 5, 2022

    @moeiscool

    this toString almost had me refactoring my app because it causes queries inside brackets to be altered.

    For Example

    select * from `Monitors` where `ke` = 'x4oIQf3IOx' and (`mid` = 'bguXu91LSS80' or `mid` = 'Xx1ioUVo6N8999' or `mid` = '3ENv8L3SwC80')
    

    suddenly becomes

    select * from `Monitors` where `ke` = 'x4oIQf3IOx' and (`mid` = 'bguXu91LSS80' and `mid` = 'Xx1ioUVo6N8999' and `mid` = '3ENv8L3SwC80')
    

    This is really bad. I'm not sure how this could be allowed to happen :(

  4. mercmobily commented on Oct 25, 2025

    @mercmobily
    Collaborator

    Hi! Thanks everyone for the thoughtful discussion.

    TL;DR:

    Action:I think we should investigate whether we should deprecate toString()/toQuery() in favor of a safer toDebugString() for logging, and update docs to steer users to toSQL().toNative() for runnable SQL and clarify orWhere object semantics (AND vs OR) to prevent logic mistakes. The rest of the comment has details.

    So... a few concrete (longer) points and follow‑ups based on the thread and code:

    toString()/toQuery() are for debugging/logging, not for producing runnable SQL for execution. Prefer:

    • builder.toSQL().toNative() to get { sql, bindings } positioned for the driver (use this for execution or when handing to a driver directly), and
    • builder.toString() only for logs.

    Query/Raw compilers attach toNative() on their compiled objects (lib/query/querycompiler.js:60–104, lib/raw.js:118–128).

    • Naming concern (ack @loujaybee). Overriding the inherited toString() in a JS API is easy to misread and can surprise people who expect a generic representation. That alone is a good argument to deprecate toString()/toQuery() and add a clearer toDebugString() alias for logging. We can warn on the old names for a deprecation window and steer docs/examples to the new method. IMPORTANT: See Document dangers of using knex.raw with query.toString #5882 with a similar request.

    • Boolean logic confusion (ack @moeiscool). toString() itself doesn’t rewrite SQL or change OR to AND; it prints the compiled SQL. The AND‑inside‑parentheses you saw comes from how object‑form orWhere works in Knex: it expands an object’s keys with AND inside a grouped clause. Concretely (lib/query/querybuilder.js:493–518), orWhere(obj) wraps a group and emits andWhere(key, value) for each key, so:

      • orWhere({ mid: 'A', other: 'B' }) compiles to ... OR ("mid" = 'A' AND "other" = 'B').
        If you want OR between the same column values, use either:
      • explicit chaining: where(function () { this.where('mid', 'A').orWhere('mid', 'B').orWhere('mid', 'C') }), or
      • whereIn('mid', ['A', 'B', 'C']), which is usually clearer and generates ... AND "mid" in ('A','B','C').
    • Docs nudge. Two small clarifications would reduce future confusion:

      • Near the “Where” section, state that object‑form orWhere({ a: 1, b: 2 }) produces a grouped AND of keys; use chaining or whereIn for OR semantics.
      • Near toString()/toQuery(), add a visible warning that they are for debugging and that toSQL().toNative() is the API for runnable SQL.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions