ronantakizawa/codereview-bench huggingface dataset:
Repository: sequelize/sequelize
File: packages/core/src/sequelize-typescript.ts (TypeScript)

Code before:
    }
  }

  /**
   * Alias of {@link AbstractQueryInterface#createSchema}
   *
   * @param schema Name of the schema
   * @param options
   */
  async createSchema(schema: string, options?: CreateSchemaOptions): Promise<void> {
    return this.queryInterface.createSchema(schema, options);
  }

  /**
   * Alias of {@link AbstractQueryInterface#listSchemas}
   *
   * @param options
   */
  async listSchemas(options?: QiListSchemasOptions) {
    return this.queryInterface.listSchemas(options);
  }

  /**
   * Alias of {@link AbstractQueryInterface#showAllSchemas}
   *
   * @deprecated Use {@link Sequelize#listSchemas} instead
   * @param options
   */
  async showAllSchemas(options?: QueryRawOptions) {
    showAllToListSchemas();

    return this.queryInterface.listSchemas(options);
  }

  /**
   * Alias of {@link AbstractQueryInterface#dropSchema}
   *
   * @param schema
   * @param options
   */
  async dropSchema(schema: string, options?: QueryRawOptions) {
    return this.queryInterface.dropSchema(schema, options);
  }

  /**
   * Alias of {@link AbstractQueryInterface#dropAllSchemas}
   *
   * @param options
   */
  async dropAllSchemas(options?: QiDropAllSchemasOptions) {
    return this.queryInterface.dropAllSchemas(options);

Review comment (bug):
```suggestion
   * @deprecated Use {@link AbstractQueryInterface#listSchemas} instead
```

I think we want to reduce the amount of methods exposed in the Sequelize class and use QueryInterface instead (so you can remove `Sequelize#listSchemas`). Correct me if I'm wrong @ephys 

Code after:
    }
  }

  /**
   * Alias of {@link AbstractQueryInterface#createSchema}
   *
   * @param schema Name of the schema
   * @param options
   */
  async createSchema(schema: string, options?: CreateSchemaOptions): Promise<void> {
    return this.queryInterface.createSchema(schema, options);
  }

  /**
   * Alias of {@link AbstractQueryInterface#showAllSchemas}
   *
   * @deprecated Use {@link AbstractQueryInterface#listSchemas} instead
   * @param options
   */
  async showAllSchemas(options?: QiListSchemasOptions) {
    showAllToListSchemas();

    return this.queryInterface.listSchemas(options);
  }

  /**
   * Alias of {@link AbstractQueryInterface#dropSchema}
   *
   * @param schema
   * @param options
   */
  async dropSchema(schema: string, options?: QueryRawOptions) {
    return this.queryInterface.dropSchema(schema, options);
  }

  /**
   * Alias of {@link AbstractQueryInterface#dropAllSchemas}
   *
   * @param options
   */
  async dropAllSchemas(options?: QiDropAllSchemasOptions) {
    return this.queryInterface.dropAllSchemas(options);
  }

  /**
   * Throws if the database version hasn't been loaded yet.
   * It is automatically loaded the first time Sequelize connects to your database.
   *
   * You can use {@link Sequelize#authenticate} to cause a first connection.
   *
   * @returns current version of the dialect that is internally loaded
Repository: sequelize/sequelize
File: packages/core/src/sequelize-typescript.ts (TypeScript)

Code before:
  /**
   * Alias of {@link AbstractQueryInterface#createSchema}
   *
   * @param schema Name of the schema
   * @param options
   */
  async createSchema(schema: string, options?: CreateSchemaOptions): Promise<void> {
    return this.queryInterface.createSchema(schema, options);
  }

  /**
   * Alias of {@link AbstractQueryInterface#listSchemas}
   *
   * @param options
   */
  async listSchemas(options?: QiListSchemasOptions) {
    return this.queryInterface.listSchemas(options);
  }

  /**
   * Alias of {@link AbstractQueryInterface#showAllSchemas}
   *
   * @deprecated Use {@link Sequelize#listSchemas} instead
   * @param options
   */
  async showAllSchemas(options?: QueryRawOptions) {
    showAllToListSchemas();

    return this.queryInterface.listSchemas(options);
  }

  /**
   * Alias of {@link AbstractQueryInterface#dropSchema}
   *
   * @param schema
   * @param options
   */
  async dropSchema(schema: string, options?: QueryRawOptions) {
    return this.queryInterface.dropSchema(schema, options);
  }

  /**
   * Alias of {@link AbstractQueryInterface#dropAllSchemas}
   *
   * @param options
   */
  async dropAllSchemas(options?: QiDropAllSchemasOptions) {
    return this.queryInterface.dropAllSchemas(options);
  }

  /**

Review comment (suggestion):
```suggestion
  async showAllSchemas(options?: QiListSchemasOptions) {
```

Code after:
  /**
   * Alias of {@link AbstractQueryInterface#createSchema}
   *
   * @param schema Name of the schema
   * @param options
   */
  async createSchema(schema: string, options?: CreateSchemaOptions): Promise<void> {
    return this.queryInterface.createSchema(schema, options);
  }

  /**
   * Alias of {@link AbstractQueryInterface#showAllSchemas}
   *
   * @deprecated Use {@link AbstractQueryInterface#listSchemas} instead
   * @param options
   */
  async showAllSchemas(options?: QiListSchemasOptions) {
    showAllToListSchemas();

    return this.queryInterface.listSchemas(options);
  }

  /**
   * Alias of {@link AbstractQueryInterface#dropSchema}
   *
   * @param schema
   * @param options
   */
  async dropSchema(schema: string, options?: QueryRawOptions) {
    return this.queryInterface.dropSchema(schema, options);
  }

  /**
   * Alias of {@link AbstractQueryInterface#dropAllSchemas}
   *
   * @param options
   */
  async dropAllSchemas(options?: QiDropAllSchemasOptions) {
    return this.queryInterface.dropAllSchemas(options);
  }

  /**
   * Throws if the database version hasn't been loaded yet.
   * It is automatically loaded the first time Sequelize connects to your database.
   *
   * You can use {@link Sequelize#authenticate} to cause a first connection.
   *
   * @returns current version of the dialect that is internally loaded
   */
  getDatabaseVersion(): string {
    if (this.options.databaseVersion == null) {
Repository: sequelize/sequelize
File: packages/core/src/import-models.ts (TypeScript)

Code before:
 * @param modelMatch
 */
export async function importModels(globPaths: string | string[], modelMatch?: ModelMatch): Promise<ModelStatic[]> {
  if (Array.isArray(globPaths)) {
    const promises: Array<Promise<ModelStatic[]>> = [];

    for (const globPath of globPaths) {
      promises.push(importModels(globPath, modelMatch));
    }

    return uniq((await Promise.all(promises)).flat(1));
  }

  const promises: Array<Promise<ModelStatic[]>> = [];
  for (const path of await glob(globPaths)) {
    const url = pathToFileURL(path).href;
    promises.push(importModelNoGlob(url, modelMatch));
  }

  return uniq((await Promise.all(promises)).flat(1));
}

async function importModelNoGlob(url: string, modelMatch?: ModelMatch): Promise<ModelStatic[]> {
  const module = await import(url);
  if (module.default && !isModelStatic(module.default) && typeof module.default === 'object') {
    Object.assign(module, module.default);
  }

  return Object.keys(module)
    .filter(exportName => {
      if (!isModelStatic(module[exportName])) {
        return false;
      }

      if (modelMatch) {
        return modelMatch(url, exportName, module[exportName]);
      }

      return true;
    })
    .map(exportName => module[exportName]);
}


Review comment (suggestion):
modules are typically sealed, so you need to create a copy like below:

![image](https://github.com/sequelize/sequelize/assets/1280915/6d44a317-a6e6-4df1-b835-d30c4b5ae991)

```suggestion
    module = { ...module, ...module.default };
```

Code after:
 * @param globPaths
 * @param modelMatch
 */
export async function importModels(globPaths: string | string[], modelMatch?: ModelMatch): Promise<ModelStatic[]> {
  if (Array.isArray(globPaths)) {
    const promises: Array<Promise<ModelStatic[]>> = [];

    for (const globPath of globPaths) {
      promises.push(importModels(globPath, modelMatch));
    }

    return uniq((await Promise.all(promises)).flat(1));
  }

  const promises: Array<Promise<ModelStatic[]>> = [];
  for (const path of await glob(globPaths)) {
    const url = pathToFileURL(path).href;
    promises.push(importModelNoGlob(url, modelMatch));
  }

  return uniq((await Promise.all(promises)).flat(1));
}

async function importModelNoGlob(url: string, modelMatch?: ModelMatch): Promise<ModelStatic[]> {
  let module = await import(url);
  // When importing a CJS file, sometimes only the default export is available,
  // as named exports depend on the file's exports being statically analyzable by node.
  // The default export contains the contents of the file's `module.exports`
  if (module.default && isPlainObject(module.default)) {
    module = { ...module.default, ...module };
  }

  return Object.keys(module)
    .filter(exportName => {
      if (!isModelStatic(module[exportName])) {
        return false;
      }

      if (modelMatch) {
        return modelMatch(url, exportName, module[exportName]);
      }

      return true;
    })
    .map(exportName => module[exportName]);
}
Repository: sequelize/sequelize
File: packages/core/src/dialects/mariadb/index.ts (TypeScript)

Code before:
  readonly connectionManager: MariaDbConnectionManager;
  readonly queryInterface: MariaDbQueryInterface;

  readonly Query = MariaDbQuery;

  constructor(sequelize: Sequelize) {
    super(sequelize, DataTypes, 'mariadb');
    this.connectionManager = new MariaDbConnectionManager(this, sequelize);
    this.queryGenerator = new MariaDbQueryGenerator({
      dialect: this,
      sequelize,
    });
    this.queryInterface = new MariaDbQueryInterface(
      sequelize,
      this.queryGenerator,
    );

    registerMariaDbDbDataTypeParsers(this);
  }

  createBindCollector() {
    return createUnspecifiedOrderedBindCollector();
  }

  escapeString(value: string) {
    return escapeMariaDbString(value);
  }

  canBackslashEscape() {
    return true;
  }

  getDefaultSchema(): string {
    return this.sequelize.options.database ?? '';
  }

  static getDefaultPort() {
    return 3306;
  }
}


Review comment (security):
considering it's the same function, I wouldn't duplicate the code. Maybe rename the function to `escapeMysqlMariaDbString` instead?

Code after:
  readonly connectionManager: MariaDbConnectionManager;
  readonly queryInterface: MariaDbQueryInterface;

  readonly Query = MariaDbQuery;

  constructor(sequelize: Sequelize) {
    super(sequelize, DataTypes, 'mariadb');
    this.connectionManager = new MariaDbConnectionManager(this, sequelize);
    this.queryGenerator = new MariaDbQueryGenerator({
      dialect: this,
      sequelize,
    });
    this.queryInterface = new MariaDbQueryInterface(
      sequelize,
      this.queryGenerator,
    );

    registerMariaDbDbDataTypeParsers(this);
  }

  createBindCollector() {
    return createUnspecifiedOrderedBindCollector();
  }

  escapeString(value: string) {
    return escapeMysqlMariaDbString(value);
  }

  canBackslashEscape() {
    return true;
  }

  getDefaultSchema(): string {
    return this.sequelize.options.database ?? '';
  }

  static getDefaultPort() {
    return 3306;
  }
}
Repository: sequelize/sequelize
File: packages/core/src/expression-builders/base-sql-expression.ts (TypeScript)

Code before:
import type { AssociationPath } from './association-path.js';
import type { Attribute } from './attribute.js';
import type { Cast } from './cast.js';
import type { Col } from './col.js';
import type { DialectAwareFn } from './dialect-aware-fn.js';
import type { Fn } from './fn.js';
import type { Identifier } from './identifier.js';
import type { JsonPath } from './json-path.js';
import type { List } from './list.js';
import type { Literal } from './literal.js';
import type { Value } from './value.js';
import type { Where } from './where.js';

/**
 * A symbol that can be used as the key for a static property on a BaseSqlExpression class to uniquely identify it.
 */
export const SQL_IDENTIFIER = Symbol('SQL_IDENTIFIER');

/**
 * Utility functions for representing SQL functions, and columns that should be escaped.
 * Please do not use these functions directly, use Sequelize.fn and Sequelize.col instead.
 *
 * @private
 */
export class BaseSqlExpression {
  declare static readonly [SQL_IDENTIFIER]: string;
}

export type DynamicSqlExpression =
  | List
  | Value
  | Identifier
  | Attribute
  | Fn
  | DialectAwareFn
  | Col
  | Cast
  | Literal
  | Where
  | JsonPath
  | AssociationPath;


Review comment (suggestion):
I'd rather not add a runtime property for something that is purely typing

You could use the same approach we've used for branding other types, by using something like

```
declare const SQL_IDENTIFIER: unique symbol;
```

Code after:
import type { AssociationPath } from './association-path.js';
import type { Attribute } from './attribute.js';
import type { Cast } from './cast.js';
import type { Col } from './col.js';
import type { DialectAwareFn } from './dialect-aware-fn.js';
import type { Fn } from './fn.js';
import type { Identifier } from './identifier.js';
import type { JsonPath } from './json-path.js';
import type { List } from './list.js';
import type { Literal } from './literal.js';
import type { Value } from './value.js';
import type { Where } from './where.js';

/**
 * A symbol that can be used as the key for a static property on a BaseSqlExpression class to uniquely identify it.
 */
export declare const SQL_IDENTIFIER: unique symbol;

/**
 * Utility functions for representing SQL functions, and columns that should be escaped.
 * Please do not use these functions directly, use Sequelize.fn and Sequelize.col instead.
 *
 * @private
 */
export class BaseSqlExpression {
  protected declare readonly [SQL_IDENTIFIER]: string;
}

export type DynamicSqlExpression =
  | List
  | Value
  | Identifier
  | Attribute
  | Fn
  | DialectAwareFn
  | Col
  | Cast
  | Literal
  | Where
  | JsonPath
  | AssociationPath;
Repository: sequelize/sequelize
File: packages/core/src/dialects/abstract/data-types.ts (TypeScript)

Code before:
    if (typeof value !== 'string') {
      ValidationErrorItem.throwDataTypeValidationError(
        util.format('%s is not a valid string', value),
      );
    }
  }
}

export interface NumberOptions {
  /**
   * Pad the value with zeros to the specified length.
   *
   * Currently useless for types that are returned as JS BigInts or JS Numbers.
   */
  // TODO: When a number is 0-filled, return it as a string instead of number or bigint
  zerofill?: boolean | undefined;

  /**
   * Is unsigned?
   */
  unsigned?: boolean | undefined;
}

export interface IntegerOptions extends NumberOptions {
  /**
   * In MariaDb: When specified, and {@link zerofill} is set, the returned value will be padded with zeros to the specified length.
   * In MySQL: This option is ignored.
   * This option is supported in no other dialect.
   * Currently useless for types that are returned as JS BigInts or JS Numbers.
   */
  length?: number;
}

export interface DecimalNumberOptions extends NumberOptions {
  /**
   * Total number of digits.
   *
   * {@link DecimalNumberOptions#scale} must be specified if precision is specified.
   */
  precision?: number | undefined;

  /**
   * Count of decimal digits in the fractional part.
   *
   * {@link DecimalNumberOptions#precision} must be specified if scale is specified.
   */
  scale?: number | undefined;
}

type AcceptedNumber = number | bigint | boolean | string | null;


Review comment (suggestion):
```suggestion
   * In MariaDB: When specified, and {@link zerofill} is set, the returned value will be padded with zeros to the specified length.
```


Code after:
      ValidationErrorItem.throwDataTypeValidationError(
        util.format('%s is not a valid string', value),
      );
    }
  }
}

export interface NumberOptions {
  /**
   * Pad the value with zeros to the specified length.
   *
   * Currently useless for types that are returned as JS BigInts or JS Numbers.
   */
  // TODO: When a number is 0-filled, return it as a string instead of number or bigint
  zerofill?: boolean | undefined;

  /**
   * Is unsigned?
   */
  unsigned?: boolean | undefined;
}

export interface IntegerOptions extends NumberOptions {
  /**
   * In MariaDB: When specified, and {@link zerofill} is set, the returned value will be padded with zeros to the specified length.
   * In MySQL: This option is ignored.
   * This option is supported in no other dialect.
   * Currently useless for types that are returned as JS BigInts or JS Numbers.
   */
  length?: number;
}

export interface DecimalNumberOptions extends NumberOptions {
  /**
   * Total number of digits.
   *
   * {@link DecimalNumberOptions#scale} must be specified if precision is specified.
   */
  precision?: number | undefined;

  /**
   * Count of decimal digits in the fractional part.
   *
   * {@link DecimalNumberOptions#precision} must be specified if scale is specified.
   */
  scale?: number | undefined;
}

type AcceptedNumber = number | bigint | boolean | string | null;

/**
Repository: sequelize/sequelize
File: packages/core/src/dialects/mysql/query-interface.js (JavaScript)

Code before:
'use strict';

import { getObjectFromMap } from '../../utils/object';
import { assertNoReservedBind, combineBinds } from '../../utils/sql';

const { AbstractQueryInterface } = require('../abstract/query-interface');
const { QueryTypes } = require('../../query-types');

/**
 * The interface that Sequelize uses to talk with MySQL/MariaDb database
 */
export class MySqlQueryInterface extends AbstractQueryInterface {
  /**
   * A wrapper that fixes MySQL's inability to cleanly remove columns from existing tables if they have a foreign key constraint.
   *
   * @override
   */
  async removeColumn(tableName, columnName, options) {
    const foreignKeys = await this.showConstraints(tableName, {
      ...options,
      columnName,
      constraintType: 'FOREIGN KEY',
    });
    await Promise.all(
      foreignKeys.map(constraint =>
        this.removeConstraint(tableName, constraint.constraintName, options),
      ),
    );

    await super.removeColumn(tableName, columnName, options);
  }

  /**
   * @override
   */

Review comment (suggestion):
```suggestion
 * The interface that Sequelize uses to talk with MySQL/MariaDB database
```


Code after:
'use strict';

import { getObjectFromMap } from '../../utils/object';
import { assertNoReservedBind, combineBinds } from '../../utils/sql';

const { AbstractQueryInterface } = require('../abstract/query-interface');
const { QueryTypes } = require('../../query-types');

/**
 * The interface that Sequelize uses to talk with MySQL/MariaDB database
 */
export class MySqlQueryInterface extends AbstractQueryInterface {
  /**
   * A wrapper that fixes MySQL's inability to cleanly remove columns from existing tables if they have a foreign key constraint.
   *
   * @override
   */
  async removeColumn(tableName, columnName, options) {
    const foreignKeys = await this.showConstraints(tableName, {
      ...options,
      columnName,
      constraintType: 'FOREIGN KEY',
    });
    await Promise.all(
      foreignKeys.map(constraint =>
        this.removeConstraint(tableName, constraint.constraintName, options),
      ),
    );

    await super.removeColumn(tableName, columnName, options);
  }

  /**
   * @override
   */
Repository: sequelize/sequelize
File: packages/core/src/model.d.ts (TypeScript)

Code before:
  /**
   * Should each row be subject to validation before it is inserted.
   * The whole insert will fail if one row fails validation
   *
   * @default false
   */
  validate?: boolean;

  /**
   * Run before / after create hooks for each individual Instance?
   * BulkCreate hooks will still be run if {@link BulkCreateOptions.hooks} is true.
   *
   * @default false
   */
  individualHooks?: boolean;

  /**
   * Ignore duplicate values for primary keys?
   *
   * @default false
   */
  ignoreDuplicates?: boolean;

  /**
   * Fields to update if row key already exists (on duplicate key update)? (only supported by MySQL,
   * MariaDb, SQLite >= 3.24.0 & Postgres >= 9.5).
   */
  updateOnDuplicate?: Array<keyof TAttributes>;

  /**
   * Include options. See `find` for details
   */
  include?: AllowArray<Includeable>;

  /**
   * Return all columns or only the specified columns for the affected rows (only for postgres)
   */
  returning?: boolean | Array<keyof TAttributes | Literal | Col>;

  /**
   * An optional parameter to specify a where clause for partial unique indexes
   * (note: `ON CONFLICT WHERE` not `ON CONFLICT DO UPDATE WHERE`).
   * Only supported in Postgres >= 9.5 and sqlite >= 9.5
   */
  conflictWhere?: WhereOptions<TAttributes>;
  /**
   * Optional override for the conflict fields in the ON CONFLICT part of the query.
   * Only supported in Postgres >= 9.5 and SQLite >= 3.24.0
   */
  conflictAttributes?: Array<keyof TAttributes>;
}

Review comment (suggestion):
```suggestion
   * MariaDB, SQLite >= 3.24.0 & Postgres >= 9.5).
```


Code after:
  /**
   * Should each row be subject to validation before it is inserted.
   * The whole insert will fail if one row fails validation
   *
   * @default false
   */
  validate?: boolean;

  /**
   * Run before / after create hooks for each individual Instance?
   * BulkCreate hooks will still be run if {@link BulkCreateOptions.hooks} is true.
   *
   * @default false
   */
  individualHooks?: boolean;

  /**
   * Ignore duplicate values for primary keys?
   *
   * @default false
   */
  ignoreDuplicates?: boolean;

  /**
   * Fields to update if row key already exists (on duplicate key update)? (only supported by MySQL,
   * MariaDB, SQLite >= 3.24.0 & Postgres >= 9.5).
   */
  updateOnDuplicate?: Array<keyof TAttributes>;

  /**
   * Include options. See `find` for details
   */
  include?: AllowArray<Includeable>;

  /**
   * Return all columns or only the specified columns for the affected rows (only for postgres)
   */
  returning?: boolean | Array<keyof TAttributes | Literal | Col>;

  /**
   * An optional parameter to specify a where clause for partial unique indexes
   * (note: `ON CONFLICT WHERE` not `ON CONFLICT DO UPDATE WHERE`).
   * Only supported in Postgres >= 9.5 and sqlite >= 9.5
   */
  conflictWhere?: WhereOptions<TAttributes>;
  /**
   * Optional override for the conflict fields in the ON CONFLICT part of the query.
   * Only supported in Postgres >= 9.5 and SQLite >= 3.24.0
   */
  conflictAttributes?: Array<keyof TAttributes>;
}