fzaninotto / Faker

Faker is a PHP library that generates fake data for you
MIT License
26.78k stars 3.58k forks source link

Null is considered unique value to Faker! #1441

Closed amrdoe closed 5 years ago

amrdoe commented 6 years ago

When using optional() in conjucation with unique(), a null is generated just once.

How to reproduce (reproduction rate of 100%):

$faker = Faker\Factory::create();
$faker->unique()->optional(0)->word; // null
$faker->unique()->optional(0)->word; // throws OverflowException

BTW $faker->optional()->unique() yields null most of the time which I find a little bit awkward!

ppelgrims commented 6 years ago

From the readme:

// optional() accepts a weight argument to specify the probability of receiving the default value.
// 0 will always return the default value; 1 will always return the provider. Default weight is 0.5 (50% chance).
// optional() accepts a default argument to specify the default value to return.
// Defaults to NULL.

Does that help?

amrdoe commented 6 years ago

The problem is null is considered a value that is unique although null essentially means "undetermined value". Even databases that have nullable unique fields allow multiple instances of null.

ppelgrims commented 6 years ago

I see what you're getting at, I had something else in mind when I wrote that reply. What do you propose as a solution?

amrdoe commented 6 years ago

That the Faker\UniqueGenerator will refrain from checking uniqueness when the value is null.

marjetika commented 6 years ago

I agree with @AmrIKhudair

jdferreira commented 5 years ago

I know this is a bit old, but I support this change as well.

pimjansen commented 5 years ago

@fzaninotto any reasonf or this since it actually functionality wise looks like a bug. However it could have bad sidefx when we will patch this since it will break probably

fzaninotto commented 5 years ago

the two modifiers are independent, and there is no way to know that one is used in conjunction with the other. Therefore I don't think that we have a way to fix this. Do we?

pimjansen commented 5 years ago

Correct, wanted to know for sure. Think we can highlight this a little in the docs for the end users then in this case

pimjansen commented 5 years ago

Did some more digging and actually what faker does here is 100% correct. If you ask for an unique record then null is still a valid record and therefor not unique.

I do not really support the idea of skipping null on uniqueness where its a valid value itself.

jdferreira commented 5 years ago

I still feel that the Faker library should support faking a unique value that is also nullable.

Consider the database schema below:

CREATE TABLE client (
    id INTEGER AUTO_INCREMENT PRIMARY KEY,
    vat VARCHAR(16) UNIQUE
    -- other fields
);

Clients needs a unique vat identifier, but the database does not need to know that information for all clients, since some clients do not have one, of fail to report, or whatever.

To properly fake an instance for this table, this is what I currently do:

$faker = Faker\Factory::create();

$client = [
    'vat' => $faker->randomFloat() < 0.9 ? $faker->unique()->randomNumber(9, true) : null,
    // other fields
];

While this works, it would be more expressive to be able to join the two Faker modifiers into $faker->optional(0.9)->unique()->randomNumber(9, true). (Furthermore, randomFloat rounds to a random number of decimal digits, while the optional modifiers does not perform that rounding operation.)

I get that this might be a niche need, and the implementation seems to be far from trivial (from what I could gather from reading some of the source code). Would a uniqueOrOptional($weight, $default) modifier be acceptable?

In any case, if you feel that closing the issue is the right approach, I'll respect and stop nagging.