Skip to content

Add synonym-aware name resolution to SqlFormatter with SQL-style SqlSynonym semantics and formatter-level synonym overrides#156

Open
Copilot wants to merge 14 commits into
masterfrom
copilot/implement-synonym-parsing
Open

Add synonym-aware name resolution to SqlFormatter with SQL-style SqlSynonym semantics and formatter-level synonym overrides#156
Copilot wants to merge 14 commits into
masterfrom
copilot/implement-synonym-parsing

Conversation

Copy link
Copy Markdown

Copilot AI commented May 12, 2026

SqlFormatter previously escaped object names directly, which prevented transparent use of database synonyms in an engine-agnostic way. This change adds formatter-level extension points and a built-in synonym service so aliases can be resolved during SQL name parsing/escaping.

  • Formatter extension point for name resolution

    • Added SqlFormatter.resolvingName (sync event emitter) and instance access to it.
    • escapeName() and escapeEntity() now emit a resolution event before ObjectNameValidator.escape(...).
    • Event payload includes target (the formatter instance) and current name.
  • Formatter-level synonym override support

    • Added optional SqlFormatter.synonyms resolver support (resolve(name)).
    • When present, formatter instance synonyms are applied first in escapeName() / escapeEntity(), before static SqlFormatter.resolvingName handlers.
    • This enables per-formatter synonym scoping while preserving global resolver extensibility.
  • Map-based singleton synonym service

    • SqlSynonym now extends Map<string, string> and acts as a singleton service via SqlSynonym.getInstance().
    • Synonym operations are performed through Map instance methods (set, delete, clear, get) plus resolve.
    • SqlSynonym constructor follows Map initialization semantics and accepts optional entries (batch initialization via new SqlSynonym(entries)).
    • resolve() supports both exact and qualified-prefix replacement (e.g. Products.ProductID) using single-pass longest-prefix matching.
    • Mapping direction follows SQL synonym usage: synonym -> original object (e.g. Products -> sales.Products).
  • Public API / typings

    • Exported SqlSynonym from src/index.js and src/index.d.ts.
    • Updated TypeScript declarations for:
      • SqlFormatter.resolvingName payload shape (target, name)
      • SqlFormatter.synonyms resolver property
      • SqlSynonym as a Map subclass with getInstance() access and Map-compatible constructor overloads.
  • Focused behavior coverage

    • Added spec/SqlSynonym.spec.js coverage for:
      • synonym substitution in SQL SELECT formatting
      • qualified object name substitution in escapeEntity / escapeName
      • propagation to formatter subclasses (OpenDataQueryFormatter)
      • constructor-based batch initialization from entry arrays
      • SQL-style synonym direction (synonym -> original)
      • formatter instance synonym precedence over static/global resolving handlers
import { SqlFormatter, SqlSynonym } from '@themost/query';

const globalSynonyms = SqlSynonym.getInstance();
globalSynonyms.set('Products', 'sales.Products');

const formatter = new SqlFormatter();
formatter.settings.nameFormat = '`$1`';
formatter.synonyms = new SqlSynonym([
  ['Products', 'inventory.Products']
]);

formatter.escapeEntity('Products');         // `inventory`.`Products`
formatter.escapeName('Products.ProductID'); // `inventory`.`Products`.`ProductID`

Agent-Logs-Url: https://github.com/themost-framework/query/sessions/48a4fe49-749a-4721-b741-95974aa979b9

Co-authored-by: kbarbounakis <9191768+kbarbounakis@users.noreply.github.com>
Copilot AI changed the title [WIP] Implement usage of synonyms while parsing query expressions Add synonym-aware name resolution to SqlFormatter via pluggable escape-time hooks May 12, 2026
Copilot AI requested a review from kbarbounakis May 12, 2026 19:10
Comment thread src/formatter.js Outdated
}
return ObjectNameValidator.validator.escape(str, this.settings.nameFormat);
const event = {
formatter: this,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that it's better to use a different name like target instead of formatter

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated in 6e84bfb: replaced formatter with target in the resolving event payload.

Comment thread src/formatter.js Outdated
const event = {
formatter: this,
name: str,
format: this.settings.nameFormat,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SqlFormatter instance is already avaiable. We don't need to pass nameFormat

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated in 6e84bfb: removed nameFormat from the event payload and now use formatter settings directly at escape-time.

@kbarbounakis kbarbounakis marked this pull request as ready for review May 12, 2026 19:28
Agent-Logs-Url: https://github.com/themost-framework/query/sessions/648745f7-74b7-49f0-9fa1-44f2cb8aad06

Co-authored-by: kbarbounakis <9191768+kbarbounakis@users.noreply.github.com>
Copilot AI requested a review from kbarbounakis May 12, 2026 19:34
Comment thread src/formatter.js Outdated
const event = {
target: this,
name: str,
type: 'name'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that the event type is redundant because resolvingName event is an event emittier for resolving only events of this type,

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the redundant event type payload in da5be42. No UI changes (screenshot N/A).

Comment thread src/sql-synonym.d.ts Outdated
@@ -0,0 +1,7 @@
export declare class SqlSynonym {
static add(name: string, synonym: string): void;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A method for passing an array of synonyms would be very useful e.g.

SqlSynonym.add([
  [ 'object1', 'synonym1' ],
  [ 'object2', 'synonym2' ]
])

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added batch array support to SqlSynonym.add() in da5be42. No UI changes (screenshot N/A).

Comment thread src/sql-synonym.js Outdated

const synonyms = new Map();

class SqlSynonym {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that SqlSynonym is a subclass of Map. It would be better to override its methods and use use Map type declarations.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactored SqlSynonym to extend Map with overridden behavior in da5be42 (plus follow-up alignment in e6e67a1). No UI changes (screenshot N/A).

Comment thread src/sql-synonym.js Outdated

const synonyms = new Map();

class SqlSynonym {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SqlSynonym acts like a service and it seems that it's better to use a static method like SqlSynonym.getInstance() and the a static instance of Synonym class.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added singleton access via SqlSynonym.getInstance() and static instance usage in da5be42 (hardened in 3179dfb). No UI changes (screenshot N/A).

Copilot AI changed the title Add synonym-aware name resolution to SqlFormatter via pluggable escape-time hooks Add synonym-aware name resolution to SqlFormatter and enhance SqlSynonym with Map-based singleton/batch APIs May 12, 2026
Copilot AI requested a review from kbarbounakis May 12, 2026 19:55
Comment thread src/sql-synonym.js Outdated
return this.instance;
}

static add(name, synonym) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems static methods lile add, remove, get etc are redundant. The static instance can be access using SqlSynonym.getInstance() without calling a static method.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in cbfc2dc: removed redundant static proxy methods and now use SqlSynonym.getInstance() directly. No UI changes (screenshot N/A).

Copilot AI requested a review from kbarbounakis May 12, 2026 20:03
Comment thread src/sql-synonym.js Outdated
this.sortedKeys = null;
}

add(name, synonym) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's try to remove add method to reduce complexity and update SqlSynonym constructor to follow Map type definition where new Map() can be called using an array of entries

interface MapConstructor {
    new(): Map<any, any>;
    new <K, V>(entries?: readonly (readonly [K, V])[] | null): Map<K, V>;
    readonly prototype: Map<any, any>;
}

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented in 9f308a3: removed the custom add() method and updated SqlSynonym to support new SqlSynonym(entries) constructor usage aligned with Map initialization. No UI changes (screenshot N/A).

Comment thread src/sql-synonym.js Outdated
if (typeof exactMatch === 'string') {
return exactMatch;
}
const keys = this.getSortedKeys();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that the usage of this.getSortedKeys() increases the complexitity of this method because every time that SqlSynonym should sort the keys.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented in 9f308a3: simplified resolve() by removing sorted-key caching and using a single-pass longest-prefix match over existing keys. No UI changes (screenshot N/A).

Agent-Logs-Url: https://github.com/themost-framework/query/sessions/ab666de7-8718-440b-bcf8-2a225953ac03

Co-authored-by: kbarbounakis <9191768+kbarbounakis@users.noreply.github.com>
Copilot AI changed the title Add synonym-aware name resolution to SqlFormatter and enhance SqlSynonym with Map-based singleton/batch APIs Add synonym-aware name resolution to SqlFormatter and align SqlSynonym with Map-based singleton APIs May 13, 2026
Copilot AI requested a review from kbarbounakis May 13, 2026 04:36
Comment thread src/sql-synonym.js Outdated
super(entries);
}

set(name, synonym) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SQL synonyms are being used for simplifying SQL expressions. So, if we are using a database engine which supports schemas, we would use a synonym to declare a table in another schema e.g. sales.Products as Products. In that case the synomym is the Products and the original data object name is sales.Products. The SqlSynonym approach -and the test unit of this MR- uses the original data object name as key and the synonym as value. We should try to follow SQL synonyms usage and reverse them.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented in 915f1e7 (with follow-up polish in 1f63850): synonym direction is now SQL-style (synonym -> original), and tests were updated accordingly (e.g. Products -> sales.Products). No UI changes (screenshot N/A).

Copilot AI and others added 2 commits May 13, 2026 05:45
Copilot AI changed the title Add synonym-aware name resolution to SqlFormatter and align SqlSynonym with Map-based singleton APIs Add synonym-aware name resolution to SqlFormatter and align SqlSynonym with SQL-style Map-based singleton semantics May 13, 2026
Copilot AI requested a review from kbarbounakis May 13, 2026 05:48
Comment thread src/formatter.js
constructor() {
//
this.provider = null;
this.resolvingName = SqlFormatter.resolvingName;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SqlFormatter should have an option to use their own SqlSynonym instance. This instance, if exists, might be used before using the static SqlFormatter.resolvingName

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented in 176ef4e (refined in ecf410f): SqlFormatter now supports a per-instance synonyms resolver, and it is applied before static resolvingName handlers. Added coverage to verify instance-level synonyms take precedence. No UI changes (screenshot N/A).

Copilot AI and others added 2 commits May 13, 2026 06:19
Copilot AI changed the title Add synonym-aware name resolution to SqlFormatter and align SqlSynonym with SQL-style Map-based singleton semantics Add synonym-aware name resolution to SqlFormatter with SQL-style SqlSynonym semantics and formatter-level synonym overrides May 13, 2026
Copilot AI requested a review from kbarbounakis May 13, 2026 06:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement the usage of synonyms while parsing query expressions

2 participants