Map crashes when it produces a null entry in the record

[jsternberg]

If map is used and the returned record has a null value, this causes a crash because a null column cannot be represented in the record itself.

[nathanielc]

@jsternberg Question of clarification: Is this caused when type inference determines that the property of the record is always null, i.e. the entire column is going to be null? Or is it that a single record produced a null value?

[jsternberg]

This is when the entire column is going to be null. The transpiler may have access a column incorrectly which is what caused this to be exercised. I'll construct a test that creates the error first.

[jsternberg]

My proposal to fix this is that if the type is null, we do not add it as a column since that's the only way it can be represented. If it is anything else, I add some error handling for when an invalid type is present.

[affo]

I think that it could happen also if the first value of a new column is null.

m, err := t.fn.Eval(i, cr)
			if err != nil {
				return errors.Wrap(err, "failed to evaluate map function")
			}
			if i == 0 {
				properties = m.Type().Properties()
				keys = make([]string, 0, len(properties))
				for k := range properties {
					keys = append(keys, k)
				}
				sort.Strings(keys)

				// Determine on which cols to group
				on = make(map[string]bool, len(tbl.Key().Cols()))
				for _, c := range tbl.Key().Cols() {
					on[c.Label] = t.mergeKey || execute.ContainsStr(keys, c.Label)
				}
			}

			key := groupKeyForObject(i, cr, m, on)
			builder, created := t.cache.TableBuilder(key)
			if created {
				if t.mergeKey {
					if err := execute.AddTableKeyCols(tbl.Key(), builder); err != nil {
						return err
					}
				}
				// Add columns from function in sorted order
				for _, k := range keys {
					if t.mergeKey && tbl.Key().HasCol(k) {
						continue
					}
					if _, err := builder.AddCol(flux.ColMeta{
						Label: k,
						Type:  execute.ConvertFromKind(properties[k].Nature()),
					}); err != nil {
						return err
					}
				}
			}

I think that is enough that Eval returns an object m for which a property type is invalid and it happens that it goes in a new table where map must create columns.

[nathanielc]

Not adding the column should have the same behavior of adding a column with every value as null as far as reading values goes. But a missing column will likely cause issues when we explicitly check the schema of tables. Do we need the ability to represent a null column?

[jsternberg]

@affo you make a good point there. It's weird, but it seems like map isn't using type inference to determine the column types. So that's another part of the bug. It should be using the results of type inference.

I think that changes the bug. Type inference may have worked correctly, but one of the values is null.

[nathanielc]

@affo you make a good point there. It's weird, but it seems like map isn't using type inference to determine the column types. So that's another part of the bug. It should be using the results of type inference.

Isn't this because we couldn't get type inference to work using with? And so we hacked around it by using the types of the data itself?

[nathanielc]

@aanthony1243 ☝️

[jsternberg]

I put up test cases that demonstrate the error in #1494. I'll start seeing if I can fix it and whether with causes a type inference issue.

[jsternberg]

I took a look and with does cause an issue. I think this is a bug in type inference and not with the concept of with. Basically, the type variables created by map don't get the updated information during the second round of type inference where it knows all of the attributes on map. This is similar to the return value bug where the kinds don't propagate correctly.

I'll have a workaround for that. Type inference should be used, but we'll just workaround it for now.