CsvReader skipping first (non header) record row when HasHeaderRecord set true.

[grifja]

Hi all,

not sure if this should be raised as a bug or if I am just doing something wrong with my setup for CsvReader. The last entry I could find similar to this dates back to 2015. In summary, CsvReader/CsvDataReader appears to be skipping the second row of data when HasHeaderRecord is set to true and I have ShouldSkipRecord configured; after the header record is skipped, the second row of data gets treated as the header record for all subsequent rows of data and subsequently skipped despite returning false for a ShouldSkipRecord check,

For a more verbose description;

I am using CsvReader in conjunction with CsvDataReader and SqlBulkCopy, and when I have a CsvReader set with HasHeaderRecord and I also have ShouldSkipRecord configured I am seeing the following behaviour:

1. The Header record gets included in the ShouldSkipRecord check where I have to purposefully skip this row - this has already covered by another user question so just making note of it
2. The second row (or first row of actual data if you will) gets skipped and treated as the header record for all other rows of data

If I disable HasHeaderRecord, everything works but that does not sit right and I have to ask if the above is by design, why? As noted in the below code, if I remove the HasHeaderRecord check from the skip record lambda method and set the HasHeaderRecord to false at configuration level, the below code works.

The above is using the latest public package, so 30.0.1 targeting ,net 6.0

The code in questions looks as follows:

var reader = new CsvReader(streamReader, new CsvConfiguration(CultureInfo.InvariantCulture)
{
    Delimiter = ",",
    IgnoreBlankLines = true,
    HasHeaderRecord = true,
    PrepareHeaderForMatch = h => h.Header.ToLowerInvariant(),
    ShouldSkipRecord = arg => !arg.Row.ValidateMyColumn(objectForLogRef),
});

...

var sqlBulkCopy = new SqlBulkCopy(connectionString, SqlBulkCopyOptions.TableLock);
sqlBulkCopy.EnableStreaming = true;
sqlBulkCopy.DestinationTableName = "dbo.MyBulkTest";
sqlBulkCopy.BatchSize = 10;

var dataReader = new CsvDataReader(reader);
await sqlBulkCopy.WriteToServerAsync(dataReader);

with the ValidateMyColumn method above being a static extension method looking something like:

static bool ValidateMyColumn(this IReaderRow row, SomeObjectType objectForLogRef)
{
        // if I remove the HasHeaderRecord check here and set the CsvConfig HasHeaderRecord = false
        // the code all works I would have originally expected, e.g. header row gets ignored and all othe
        // rows are included.
	if (row.Configuration.HasHeaderRecord && row.Parser.Row == 1)
	{
		return false;
	}

	// Do other checks, for example:

	if (int.TryParse(row[ordinal], out var _))
	{
		return true;
	}

	// Logging to objectForLogRef
	return false;
}

[AltruCoder]

I think you have your logic off a little bit.

!arg.Row.ValidateMyColumn(objectForLogRef)

along with

if (row.Configuration.HasHeaderRecord && row.Parser.Row == 1)
{
   return false;
}

will return true to the question of ShouldSkipRecord.

You don't want it to skip the header. That means it will skip the first row and end up reading the next row as the header row. Your code should be

if (row.Configuration.HasHeaderRecord && row.Parser.Row == 1)
{
   return true;
}

[grifja]

Thanks for the reply AltruCoder; the ! operator at the front of
!arg.Row.ValidateMyColumn(objectForLogRef) considered true/false responses as being inverted from the 'ValidateMyColumn' method. Just to avoid further confusion and to make sure I had not done something dumb I have swapped the true/false results around (and removed the ! operator from the above code) so now a true result represents skipping of a record.

The outcome I am seeing is still the same; header record skipped as expected, then second row of data treated as the header row and consequently ignored.

[AltruCoder]

The issue is not the ! operator. The issue is your code is telling CsvHelper to skip reading the first row, which has your header. Switching all the true/false responses along with the ! operator will still end up with the same result.

You need to return false to ShouldSkipRecord if it is the header row. You also need to return false if the record is valid. Only return true to ShouldSkipRecord if the record is not valid.

With your original code I get a HeaderValidationException because the code is telling CsvReader to skip the first row if you have a header.

void Main()
{
	var data = @"Id,Name
1,Name #1
2,Name #2";

	var reader = new CsvReader(new StringReader(data), new CsvConfiguration(CultureInfo.InvariantCulture)
	{
		Delimiter = ",",
		IgnoreBlankLines = true,
		HasHeaderRecord = true,
		PrepareHeaderForMatch = h => h.Header.ToLowerInvariant(),
		ShouldSkipRecord = arg => !arg.Row.ValidateMyColumn(),
	});
	
	reader.GetRecords<Foo>().ToList().Dump();
}

public static bool ValidateMyColumn(this IReaderRow row)
{
	// if I remove the HasHeaderRecord check here and set the CsvConfig HasHeaderRecord = false
	// the code all works I would have originally expected, e.g. header row gets ignored and all othe
	// rows are included.
	if (row.Configuration.HasHeaderRecord && row.Parser.Row == 1)
	{
		return false;
	}

	// Do other checks, for example:

	if (int.TryParse(row[0], out var _))
	{
		return true;
	}

	// Logging to objectForLogRef
	return false;
}

public class Foo
{
	public int Id { get; set; }
	public string Name { get; set; }
}

All you have to do is change false to true in the code where it checks for the header.

void Main()
{
	var data = @"Id,Name
1,Name #1
2,Name #2";

	var reader = new CsvReader(new StringReader(data), new CsvConfiguration(CultureInfo.InvariantCulture)
	{
		Delimiter = ",",
		IgnoreBlankLines = true,
		HasHeaderRecord = true,
		PrepareHeaderForMatch = h => h.Header.ToLowerInvariant(),
		ShouldSkipRecord = arg => !arg.Row.ValidateMyColumn(),
	});
	
	reader.GetRecords<Foo>().ToList().Dump();
}

public static bool ValidateMyColumn(this IReaderRow row)
{
	// if I remove the HasHeaderRecord check here and set the CsvConfig HasHeaderRecord = false
	// the code all works I would have originally expected, e.g. header row gets ignored and all othe
	// rows are included.
	if (row.Configuration.HasHeaderRecord && row.Parser.Row == 1)
	{
		return true;
	}

	// Do other checks, for example:

	if (int.TryParse(row[0], out var _))
	{
		return true;
	}

	// Logging to objectForLogRef
	return false;
}

public class Foo
{
	public int Id { get; set; }
	public string Name { get; set; }
}

[grifja]

Thanks for the help @AltruCoder,

the issue was the skip on the header row when validating data rows, as you explained above. I had been so intent on making sure that the header row data was not processed in the same manner as data rows to not consider that skipping the header record might have other repercussions.