Remove the default exists condition for group by keys?

[srikanthccv]

Bug description

For every group by clause, we are adding a condition to make sure that the key exists

signoz/pkg/query-service/app/logs/v3/query_builder.go

Lines 208 to 218 in 03838f5

	// add group by conditions to filter out log lines which doesn't have the key
	for _, attr := range groupBy {
	if !attr.IsColumn {
	columnType := getClickhouseLogsColumnType(attr.Type)
	columnDataType := getClickhouseLogsColumnDataType(attr.DataType)
	conditions = append(conditions, fmt.Sprintf("has(%s_%s_key, '%s')", columnType, columnDataType, attr.Key))
	} else if attr.Type != v3.AttributeKeyTypeUnspecified {
	// for materialzied columns
	conditions = append(conditions, fmt.Sprintf("%s_exists`=true", strings.TrimSuffix(getClickhouseColumnName(attr), "`")))
	}
	}

. This might not be correct in all cases. Say there are two keys service.name and ecs.service.name which are mutually exclusive i.e a log record has either one of them in attributes but not both. If I group by service.name and ecs.service.name it could return no result. Perhaps we should not add key exists condition ourselves and let the user do it if necessary.

[nityanandagohain]

So you are saying that if the user doesn't explicitly use the exists then empty values is fine ?

So are with fine with:- If there I group by service.name. And there are n logs with no service name then count of n will be added to empty service name. So it will be difficult to differentiate between empty service name and logs where service.name doesn't exists.

[srikanthccv]

So it will be difficult to differentiate between empty service name and logs where service.name doesn't exists.

In practice, this doesn't happen where service.name (or any attribute) is deliberately set to empty, and it should be distinguished from the attribute's existence. On occasions when it is important, users will need to add the EXISTS filter.

So you are saying that if the user doesn't explicitly use the exists then empty values is fine ?

The current result doesn't display anything and just discards the data, as we observed when trying to group by service.name (the total of each value in the group by doesn't match the total number of log lines). Therefore, showing an empty result is accurate and preferable.

[srikanthccv]

This should be fixed otherwise the charts are just lying.

[nityanandagohain]

sure prioritising this

[srikanthccv]

So it will be difficult to differentiate between empty service name and logs where service.name doesn't exists.

In practice, this doesn't happen where service.name (or any attribute) is deliberately set to empty, and it should be distinguished from the attribute's existence. On occasions when it is important, users will need to add the EXISTS filter.

This is a lazy assumption to make for any data type specifically expecting users to add the filter.