Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Entity generator] Overlapping FKs over a column produces first one defined #4898

Closed
boenrobot opened this issue Nov 6, 2023 · 0 comments
Closed

Comments

@boenrobot
Copy link
Collaborator

boenrobot commented Nov 6, 2023

Describe the bug

No pre-existing entities. Trying to generate some from a DB schema.

DB schema has a table where there are two (or more...) FKs, and they include the same column as part of them.

In this case, the shared column gets to be of the type of whichever FK containing that column was defined first, which is not necessarily appropriate.

The times when you'd do this type of thing is when you want the column to comply with multiple constraints, and on application level, this exposes only one of those constraints on the shared column, and the other constraints may or may not be exposed in the other properties.

To Reproduce

  1. Create a MySQL DB with this schema:
-- MySQL Workbench Forward Engineering

SET @OLD_UNIQUE_CHECKS=@@UNIQUE_CHECKS, UNIQUE_CHECKS=0;
SET @OLD_FOREIGN_KEY_CHECKS=@@FOREIGN_KEY_CHECKS, FOREIGN_KEY_CHECKS=0;
SET @OLD_SQL_MODE=@@SQL_MODE, SQL_MODE='ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_ENGINE_SUBSTITUTION';

-- -----------------------------------------------------
-- Schema overlap_fk_example
-- -----------------------------------------------------
DROP SCHEMA IF EXISTS `overlap_fk_example` ;

-- -----------------------------------------------------
-- Schema overlap_fk_example
-- -----------------------------------------------------
CREATE SCHEMA IF NOT EXISTS `overlap_fk_example` DEFAULT CHARACTER SET utf8 ;
USE `overlap_fk_example` ;

-- -----------------------------------------------------
-- Table `sellers`
-- -----------------------------------------------------
CREATE TABLE IF NOT EXISTS `sellers` (
  `seller_id` INT UNSIGNED NOT NULL AUTO_INCREMENT,
  `name` VARCHAR(255) NOT NULL,
  PRIMARY KEY (`seller_id`),
  UNIQUE INDEX `name_UNIQUE` (`name` ASC) VISIBLE)
ENGINE = InnoDB;


-- -----------------------------------------------------
-- Table `products`
-- -----------------------------------------------------
CREATE TABLE IF NOT EXISTS `products` (
  `product_id` INT UNSIGNED NOT NULL AUTO_INCREMENT,
  `name` VARCHAR(255) NOT NULL,
  `current_price` DECIMAL(10,2) NOT NULL,
  `current_quantity` INT NOT NULL DEFAULT 0,
  PRIMARY KEY (`product_id`),
  UNIQUE INDEX `name_UNIQUE` (`name` ASC) VISIBLE)
ENGINE = InnoDB;


-- -----------------------------------------------------
-- Table `product_sellers`
-- -----------------------------------------------------
CREATE TABLE IF NOT EXISTS `product_sellers` (
  `sller_id` INT UNSIGNED NOT NULL,
  `product_id` INT UNSIGNED NOT NULL,
  `is_currently_allowed` TINYINT(1) NOT NULL DEFAULT 0,
  PRIMARY KEY (`sller_id`, `product_id`),
  INDEX `fk_product_sellers_sellers_idx` (`sller_id` ASC) VISIBLE,
  CONSTRAINT `fk_product_sellers_sellers`
    FOREIGN KEY (`sller_id`)
    REFERENCES `sellers` (`seller_id`)
    ON DELETE CASCADE
    ON UPDATE CASCADE,
  CONSTRAINT `fk_product_sellers_products1`
    FOREIGN KEY (`product_id`)
    REFERENCES `products` (`product_id`)
    ON DELETE CASCADE
    ON UPDATE CASCADE)
ENGINE = InnoDB;


-- -----------------------------------------------------
-- Table `countries`
-- -----------------------------------------------------
CREATE TABLE IF NOT EXISTS `countries` (
  `code` CHAR(2) NOT NULL,
  PRIMARY KEY (`code`))
ENGINE = InnoDB;


-- -----------------------------------------------------
-- Table `product_country_map`
-- -----------------------------------------------------
CREATE TABLE IF NOT EXISTS `product_country_map` (
  `country` CHAR(2) NOT NULL,
  `product_id` INT UNSIGNED NOT NULL,
  `is_currently_allowed` TINYINT(1) NOT NULL DEFAULT 0,
  PRIMARY KEY (`country`, `product_id`),
  CONSTRAINT `fk_country_allow_list_products1`
    FOREIGN KEY (`product_id`)
    REFERENCES `products` (`product_id`)
    ON DELETE NO ACTION
    ON UPDATE NO ACTION,
  CONSTRAINT `fk_product_country_map_countries1`
    FOREIGN KEY (`country`)
    REFERENCES `countries` (`code`)
    ON DELETE NO ACTION
    ON UPDATE NO ACTION)
ENGINE = InnoDB;


-- -----------------------------------------------------
-- Table `sales`
-- -----------------------------------------------------
CREATE TABLE IF NOT EXISTS `sales` (
  `sale_id` INT UNSIGNED NOT NULL AUTO_INCREMENT,
  `country` CHAR(2) NOT NULL,
  `sller_id` INT UNSIGNED NOT NULL,
  `product_id` INT UNSIGNED NOT NULL,
  `singular_price` DECIMAL(10,2) NOT NULL,
  `quantity_sold` INT UNSIGNED NOT NULL DEFAULT 1,
  PRIMARY KEY (`sale_id`),
  INDEX `fk_sales_product_sellers1_idx` (`sller_id` ASC, `product_id` ASC) VISIBLE,
  INDEX `fk_sales_country_allow_list1_idx` (`country` ASC, `product_id` ASC) VISIBLE,
  CONSTRAINT `fk_sales_product_sellers1`
    FOREIGN KEY (`sller_id` , `product_id`)
    REFERENCES `product_sellers` (`sller_id` , `product_id`)
    ON DELETE RESTRICT
    ON UPDATE CASCADE,
  CONSTRAINT `fk_sales_product_country_map1`
    FOREIGN KEY (`country` , `product_id`)
    REFERENCES `product_country_map` (`country` , `product_id`)
    ON DELETE RESTRICT
    ON UPDATE CASCADE)
ENGINE = InnoDB;


SET SQL_MODE=@OLD_SQL_MODE;
SET FOREIGN_KEY_CHECKS=@OLD_FOREIGN_KEY_CHECKS;
SET UNIQUE_CHECKS=@OLD_UNIQUE_CHECKS;
  1. Run the entity generator
  2. Observe the output for the Sales entity in particular (the rest are fine...):
import { Entity, Index, ManyToOne, OptionalProps, PrimaryKey, PrimaryKeyProp, PrimaryKeyType, Property } from '@mikro-orm/core';
import { ProductCountryMap } from './ProductCountryMap';
import { ProductSellers } from './ProductSellers';

@Entity()
@Index({ name: 'fk_sales_country_allow_list1_idx', properties: ['country', 'productId'] })
@Index({ name: 'fk_sales_product_sellers1_idx', properties: ['sllerId', 'productId'] })
export class Sales {

  [PrimaryKeyProp]?: 'saleId';

  [PrimaryKeyType]?: number;

  [OptionalProps]?: 'quantitySold';

  @PrimaryKey()
  saleId!: number;

  @ManyToOne({ entity: () => ProductCountryMap, fieldName: 'country', onUpdateIntegrity: 'cascade' })
  country!: ProductCountryMap;

  @ManyToOne({ entity: () => ProductSellers, fieldName: 'sller_id', onUpdateIntegrity: 'cascade' })
  sllerId!: ProductSellers;

  @ManyToOne({ entity: () => ProductCountryMap, fieldName: 'country', onUpdateIntegrity: 'cascade' })
  productId!: ProductCountryMap;

  @Property({ columnType: 'numeric(10,2)' })
  singularPrice!: string;

  @Property({ default: 1 })
  quantitySold: number = 1;

}

Expected behavior

After outputting columns that are only part of one FK (be it single or composite FK), any column that is part of a composite foreign key not already rendered should be outputted as a property with a name based on the foreign key name, rather than the column name. And any column that is covered by composite FKs, all of which are already outputted, should not be outputted at all.

Also, composite foreign keys should feature the full fields involved.

In the case of the above example, this means instead of

  @ManyToOne({ entity: () => ProductCountryMap, fieldName: 'country', onUpdateIntegrity: 'cascade' })
  country!: ProductCountryMap;

  @ManyToOne({ entity: () => ProductSellers, fieldName: 'sller_id', onUpdateIntegrity: 'cascade' })
  sllerId!: ProductSellers;

  @ManyToOne({ entity: () => ProductCountryMap, fieldName: 'country', onUpdateIntegrity: 'cascade' })
  productId!: ProductCountryMap;

the generated output should be

  @ManyToOne({ entity: () => ProductCountryMap, fieldNames: ['country', 'product_id'], onUpdateIntegrity: 'cascade' })
  country!: ProductCountryMap;

  @ManyToOne({ entity: () => ProductSellers, fieldNames: ['sller_id', 'product_id'], onUpdateIntegrity: 'cascade' })
  sllerId!: ProductSellers;

(no productId property; "product_id" featured in "fieldNames" array instead of "fieldName" with only the first field name featured.)

If there was a further 3rd constraint that is only referencing Products, I would expect Sales to have a productId field that is a reference to that, even if that constraint was declared last. However, adding one, but declaring it last, produces the same output as above. Adding it first fixes the type of the productId property, while still keeping the definition of the other ones with incorrect fieldNames.

If there were additional separate constraints on seller_id and country to Sellers and Countries, I would expect the properties to point to those constraints, because those constraints only feature that one column. Then the composite keys would be rendered with property names based on the FK name. Unless a dedicated naming strategy method is introduced, I guess that would mean the properties would be called "fkSalesProductSellers1" and "fkSalesProductCountryMap1".

Additional context
Above example is a simplified reproduction out of a real schema.

Versions

Dependency Version
node 21.1.0
typescript 5.2.2
mikro-orm 5.9.2
mysql 8
@boenrobot boenrobot changed the title [Entity generator] Overlapping FKs over a column result in first one defined being picked [Entity generator] Overlapping FKs over a column produces first one defined Nov 6, 2023
boenrobot pushed a commit to boenrobot/mikro-orm that referenced this issue Nov 8, 2023
The process of generating relations is heavily altered
to output foreign key relations with property names based on the foreign key constrain name,
except in cases where the foreign key is the only foreign key involved with a certain column,
in which case, that column is used as the name of the property.

When composite foreign keys are outputted, all fields are included with "fieldNames".

In general, if a metadata property is an array, that array will be outputted correctly.

If a column is part of a foreign key (be it a composite one or ambiguous single one),
no "plain" column property is generated.
Access to the property is through the foreign key properties instead.

Fixes mikro-orm#4898.
boenrobot pushed a commit to boenrobot/mikro-orm that referenced this issue Nov 8, 2023
The process of generating relations is heavily altered
to output foreign key relations with property names based on the foreign key constrain name,
except in cases where the foreign key is the only foreign key involved with a certain column,
in which case, that column is used as the name of the property.

When composite foreign keys are outputted, all fields are included with "fieldNames".

In general, if a metadata property is an array, that array will be outputted correctly.

If a column is part of a foreign key (be it a composite one or ambiguous single one),
no "plain" column property is generated.
Access to the property is through the foreign key properties instead.

Fixes mikro-orm#4898.
boenrobot pushed a commit to boenrobot/mikro-orm that referenced this issue Nov 8, 2023
The process of generating relations is heavily altered
to output foreign key relations with property names based on the foreign key constrain name,
except in cases where the foreign key is the only foreign key involved with a certain column,
in which case, that column is used as the name of the property.

When composite foreign keys are outputted, all fields are included with "fieldNames".

In general, if a metadata property is an array, that array will be outputted correctly.

If a column is part of a foreign key (be it a composite one or ambiguous single one),
no "plain" column property is generated.
Access to the property is through the foreign key properties instead.

Made the order of indexes and FKs explicit,
based on the name and definition order of the columns,
thus ensuring a consistent output.

Fixes mikro-orm#4898.
boenrobot pushed a commit to boenrobot/mikro-orm that referenced this issue Nov 8, 2023
The process of generating relations is heavily altered
to output foreign key relations with property names based on the foreign key constrain name,
except in cases where there is one column that the FK is involved with,
in which case, that column is used as the name of the property.

When composite foreign keys are outputted, all fields are included with "fieldNames".

In general, if a metadata property is an array, that array will be outputted correctly.

If a column is part of a foreign key (be it a composite one or ambiguous single one),
no "plain" column property is generated, unless all FKs are null-able, while the column is not.
Access to the property is through the foreign key properties instead.

Made the order of indexes and FKs in the information schema select explicit,
based on the name and definition order of the columns,
thus ensuring a consistent output.

Fixes mikro-orm#4898.
boenrobot pushed a commit to boenrobot/mikro-orm that referenced this issue Nov 8, 2023
The process of generating relations is heavily altered
to output foreign key relations with property names based on the foreign key constrain name,
except in cases where there is one column that the FK is involved with,
in which case, that column is used as the name of the property.

When composite foreign keys are outputted, all fields are included with "fieldNames".

In general, if a metadata property is an array, that array will be outputted correctly.

If a column is part of a foreign key (be it a composite one or ambiguous single one),
no "plain" column property is generated, unless all FKs are null-able, while the column is not.
Access to the property is through the foreign key properties instead.

Made the order of indexes and FKs in the information schema select explicit,
based on the name and definition order of the columns,
thus ensuring a consistent output.

Fixes mikro-orm#4898.
boenrobot pushed a commit to boenrobot/mikro-orm that referenced this issue Nov 8, 2023
The process of generating relations is heavily altered
to output foreign key relations with property names based on the foreign key constrain name,
except in cases where there is one column that the FK is involved with,
in which case, that column is used as the name of the property.

When composite foreign keys are outputted, all fields are included with "fieldNames".

In general, if a metadata property is an array, that array will be outputted correctly.

If a column is part of a foreign key (be it a composite one or ambiguous single one),
no "plain" column property is generated, unless all FKs are null-able, while the column is not.
Access to the property is through the foreign key properties instead.

Made the order of indexes and FKs in the information schema select explicit,
based on the name and definition order of the columns,
thus ensuring a consistent output.

Fixes mikro-orm#4898.
boenrobot pushed a commit to boenrobot/mikro-orm that referenced this issue Nov 8, 2023
The process of generating relations is heavily altered
to output foreign key relations with property names based on the foreign key constrain name,
except in cases where there is one column that the FK is involved with,
in which case, that column is used as the name of the property.

When composite foreign keys are outputted, all fields are included with "fieldNames".

In general, if a metadata property is an array, that array will be outputted correctly.

If a column is part of a foreign key (be it a composite one or ambiguous single one),
no "plain" column property is generated, unless all FKs are null-able, while the column is not.
Access to the property is through the foreign key properties instead.

Made the order of indexes and FKs in the information schema select explicit,
based on the name and definition order of the columns,
thus ensuring a consistent output.

Fixes mikro-orm#4898.
boenrobot pushed a commit to boenrobot/mikro-orm that referenced this issue Nov 9, 2023
The process of generating relations is heavily altered
to output foreign key relations with property names based on the foreign key constrain name,
except in cases where it is safe to use a specific column name instead.

When composite foreign keys are outputted, all fields are included with "fieldNames",
unless all names involved at the same as the join columns.

In general, if a metadata property is an array, that array will be outputted correctly.

If a column is part of a foreign key (be it a composite one or ambiguous single one),
no "plain" column property is generated, unless all FKs are null-able, while the column is not.
Access to the property is through the foreign key properties instead.

Made the order of indexes and FKs in the information schema select explicit,
based on the name and definition order of the columns,
thus ensuring a consistent output.

Tweaked the OneToOne relation detection to also work for composite primary keys.
If the entire PK is covered by exactly one ManyToOne relation,
it now counts at OneToOne regardless of the number of columns involved.

Fixes mikro-orm#4898.
boenrobot pushed a commit to boenrobot/mikro-orm that referenced this issue Nov 9, 2023
The process of generating relations is heavily altered
to output foreign key relations with property names based on the foreign key constrain name,
except in cases where it is safe to use a specific column name instead.

When composite foreign keys are outputted, all fields are included with "fieldNames",
unless all names involved at the same as the join columns.

In general, if a metadata property is an array, that array will be outputted correctly.

If a column is part of a foreign key (be it a composite one or ambiguous single one),
no "plain" column property is generated, unless all FKs are null-able, while the column is not.
Access to the property is through the foreign key properties instead.

Made the order of indexes and FKs in the information schema select explicit,
based on the name and definition order of the columns,
thus ensuring a consistent output.

Tweaked the OneToOne relation detection to also work for composite primary keys.
If the entire PK is covered by exactly one ManyToOne relation,
it now counts at OneToOne regardless of the number of columns involved.

Fixes mikro-orm#4898.
boenrobot pushed a commit to boenrobot/mikro-orm that referenced this issue Nov 9, 2023
The process of generating relations is heavily altered
to output foreign key relations with property names based on the foreign key constrain name,
except in cases where it is safe to use a specific column name instead.

When composite foreign keys are outputted, all fields are included with "fieldNames",
unless all names involved at the same as the join columns.

In general, if a metadata property is an array, that array will be outputted correctly.

If a column is part of a foreign key (be it a composite one or ambiguous single one),
no "plain" column property is generated, unless all FKs are null-able, while the column is not.
Access to the property is through the foreign key properties instead.

Made the order of indexes and FKs in the information schema select explicit,
based on the name and definition order of the columns,
thus ensuring a consistent output.

Tweaked the OneToOne relation detection to also work for composite primary keys.
If the entire PK is covered by exactly one ManyToOne relation,
it now counts at OneToOne regardless of the number of columns involved.

Fixes mikro-orm#4898.
boenrobot pushed a commit to boenrobot/mikro-orm that referenced this issue Nov 9, 2023
The process of generating relations is heavily altered
to output foreign key relations with property names based on the foreign key constrain name,
except in cases where it is safe to use a specific column name instead.

When composite foreign keys are outputted, all fields are included with "fieldNames",
unless all names involved at the same as the join columns.

In general, if a metadata property is an array, that array will be outputted correctly.

If a column is part of a foreign key (be it a composite one or ambiguous single one),
no "plain" column property is generated, unless all FKs are null-able, while the column is not.
Access to the property is through the foreign key properties instead.

Made the order of indexes and FKs in the information schema select explicit,
based on the name and definition order of the columns,
thus ensuring a consistent output.

Tweaked the OneToOne relation detection to also work for composite primary keys.
If the entire PK is covered by exactly one ManyToOne relation,
it now counts at OneToOne regardless of the number of columns involved.

Fixes mikro-orm#4898.
boenrobot pushed a commit to boenrobot/mikro-orm that referenced this issue Nov 9, 2023
The process of generating relations is heavily altered
to output foreign key relations with property names based on the foreign key constrain name,
except in cases where it is safe to use a specific column name instead.

When composite foreign keys are outputted, all fields are included with "fieldNames",
unless all names involved at the same as the join columns.

In general, if a metadata property is an array, that array will be outputted correctly.

If a column is part of a foreign key (be it a composite one or ambiguous single one),
no "plain" column property is generated, unless all FKs are null-able, while the column is not.
Access to the property is through the foreign key properties instead.

Made the order of indexes and FKs in the information schema select explicit,
based on the name and definition order of the columns,
thus ensuring a consistent output.

Tweaked the OneToOne relation detection to also work for composite primary keys.
If the entire PK is covered by exactly one ManyToOne relation,
it now counts at OneToOne regardless of the number of columns involved.

Fixes mikro-orm#4898.
boenrobot pushed a commit to boenrobot/mikro-orm that referenced this issue Nov 13, 2023
The process of generating relations is heavily altered
to output foreign key relations with property names based on the foreign key constrain name,
except in cases where it is safe to use a specific column name instead.

When composite foreign keys are outputted, all fields are included with "fieldNames",
unless all names involved at the same as the join columns.

In general, if a metadata property is an array, that array will be outputted correctly.

If a column is part of a foreign key (be it a composite one or ambiguous single one),
no "plain" column property is generated, unless all FKs are null-able, while the column is not.
Access to the property is through the foreign key properties instead.

Made the order of indexes and FKs in the information schema select explicit,
based on the name and definition order of the columns,
thus ensuring a consistent output.

Tweaked the OneToOne relation detection to also work for composite primary keys.
If the entire PK is covered by exactly one ManyToOne relation,
it now counts at OneToOne regardless of the number of columns involved.

Fixes mikro-orm#4898.
boenrobot pushed a commit to boenrobot/mikro-orm that referenced this issue Nov 14, 2023
The process of generating relations is heavily altered
to output foreign key relations with property names based on the foreign key constrain name,
except in cases where it is safe to use a specific column name instead.

When composite foreign keys are outputted, all fields are included with "fieldNames",
unless all names involved at the same as the join columns.

In general, if a metadata property is an array, that array will be outputted correctly.

If a column is part of a foreign key (be it a composite one or ambiguous single one),
no "plain" column property is generated, unless all FKs are null-able, while the column is not.
Access to the property is through the foreign key properties instead.

Made the order of indexes and FKs in the information schema select explicit,
based on the name and definition order of the columns,
thus ensuring a consistent output.

Tweaked the OneToOne relation detection to also work for composite primary keys.
If the entire PK is covered by exactly one ManyToOne relation,
it now counts at OneToOne regardless of the number of columns involved.

Fixes mikro-orm#4898.
@B4nan B4nan closed this as completed in abad6ca Nov 17, 2023
boenrobot pushed a commit to boenrobot/mikro-orm that referenced this issue Nov 17, 2023
boenrobot pushed a commit to boenrobot/mikro-orm that referenced this issue Nov 17, 2023
boenrobot pushed a commit to boenrobot/mikro-orm that referenced this issue Nov 20, 2023
Also moved mikro-orm#4892 into the entity generator folder.

Moved schema creation in each file into a beforeAll(),
rather than at the start of every test.
B4nan pushed a commit that referenced this issue Nov 20, 2023
As requested in #4898

Co-authored-by: Vasil Rangelov <vasil.tech@condor-gaming.com>
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

No branches or pull requests

1 participant